GuessAuth

[untitaker]

See https://github.com/kennethreitz/requests/issues/2006

This currently breaks and i can't explain how it breaks since the same code is
used in requests.

[Lukasa]

In what way does it break?

[untitaker]

Wait for it...

[untitaker]

Bam https://travis-ci.org/sigmavirus24/requests-toolbelt/jobs/23968056

[Lukasa]

Heh, whoops, I forgot we had Travis. =D Awkward.

[untitaker]

Specifically, the handle_401 method is stolen from HTTPDigestAuth, uses the same code to copy the request but breaks due to some internal state of the request object.

[Lukasa]

Hmm. Does it break if you make a real request? I'm wondering if Betamax isn't doing the right thing with the cookies parameter.

[untitaker]

That is exactly it. Quickly glancing over betamax it doesn't seem it serializes cookies at all?

[Lukasa]

That's perfectly possible. Betamax is @sigmavirus24's baby, so I'll let him field this for now, but I'm sure he'd welcome a pull request. =)

[untitaker]

I am wondering whether just serializing r._cookies as a whole is a good solution. That didn't even make sense.

[sigmavirus24]

We don't recreate the _cookies attribute because it should really never have been added to a PreparedRequest object.

If you'd like to open a bug on betamax about this, we'll have to figure out how to properly recreate the cookie jar from the headers on a PreparedRequest. Further, we'll have to figure out where the connection gets tacked onto a Response since that seems to be yet another problem.

[untitaker]

If you'd like to open a bug on betamax about this

I seriously don't care at all. Your opinion is more valuable because you know both libraries better than i do.

[sigmavirus24]

I have to imagine that at some point someone else is going to run into this problem as well. It's definitely worth fixing.

[sigmavirus24]

I've started working on a fix here: https://gitlab.com/sigmavirus24/betamax/commits/fix-prepared-cookies (incidentally, also a refactor of sorts)

[untitaker]

Thanks. I am still curious though:

You stated that _cookies shouldn't have been added to PreparedRequest. Is this your opinion, and why do you think so?

[sigmavirus24]

So it serves a purpose but I don't like the fact that we settled on that solution. It's messy and violates the whole naming convention surrounding _ prefixed attributes -- in other words, that's supposed to be a private attribute/method and it's not meant to be "publicly" relied upon. The reason it is private is because we don't want users relying on it. The reason we violate the naming convention is because we need a good way of passing around the cookie jar for a request. Personally, I've begun looking at a way to dynamically generate the cookie jar from the headers on a prepared request as an alternative. I'm not sure it's going to be that much better.

Also, the PreparedRequest shouldn't have information on there that isn't going to actually affect the sending of it through the transport adapter.

While I'm complaining: I'm also not content in the slightest with the fact that the Transport Adapter attaches itself to a generated response as connection. I just haven't spent a great deal of time deciding how best to solve the problem that those two things solve.

[sigmavirus24]

So while these issues are fixed, we cannot have matching on the Authorization header in the instance of digest authentication. The reason is that we would need to mock out time.ctime() to consistently return the same value. If we don't, the headers will never match. The cnonce and response will always be different. I'm thinking of adding a matcher for this specifically. It would be entirely non-obvious to someone naïvely using betamax.

[untitaker]

What about just hitting the network for this test for now?

[sigmavirus24]

@untitaker betamax is fixed now. If you change the dev-requirements to point to the repo (e.g. https://gitlab.com/sigmavirus24/betamax) it should pass the tests. :)

You just need to tell Betamax to also match on digest-auth. See this for an example.

[untitaker]

Unfortunately it doesn't seem like that...

[sigmavirus24]

Sorry, forgot to mention the crucial step: You'll have to rerecord the cassette in question (i.e., rm tests/cassettes/my_cassette.json, py.test)

[untitaker]

I don't think that's the reason, at least not for Python 3.

[untitaker]

betamaxpy/betamax#32

[untitaker]

Yay it works!

[sigmavirus24]

Thanks for all your help @untitaker ! 🍰 🍰 🍻

[sigmavirus24]

In other news, you have some syntax that python 2.6/2.7 cannot handle.

[untitaker]

This seems rather related to requests 2.0.1.

[untitaker]

I don't know how to fix the errors related to requests.

[Lukasa]

I haven't looked so this is just a shot in the dark, but I suspect this might be related to the fact that _cookies field didn't exist in 2.0.1.

[untitaker]

How about we just skip these tests for Requests 2.0.1? Otherwise betamax would have to maintain Requests 2.0.1 compat, which is hardly worth it for the general usecase of betamax.

[sigmavirus24]

@untitaker this is not the fault of requests. Dropping requests 2.0.1 is worth discussion over at betamax

[untitaker]

Yes i already force-pushed a solution to this.

[untitaker]

Bump. Should i skip these tests for requests 2.0.1?

[sigmavirus24]

Hey @untitaker I lost track of this. I'm going to look at it tonight or tomorrow to determine why those tests are failing. They really shouldn't be failing at all.

[untitaker]

Ok great.

[sigmavirus24]

Ok, I'm actually getting around to this now. Sorry for the delay. I've been super busy.

[sigmavirus24]

Thanks @untitaker ✨ 🍰 ✨

[untitaker]

\o/

[sigmavirus24]

FWIW, I bumped the minimum test-supported version of requests to 2.1

[untitaker]

Is there a ETA for a new version?

[sigmavirus24]

@untitaker I think I want to get #22 in at least. I'm not sure I like #21 anymore but I'm going to leave that open for now.

[sigmavirus24]

@untitaker you never added yourself to AUTHORS, feel free to send a PR or let me know how you want to be credited.

[untitaker]

Submitted at #65