Pass user options to hooks

Hooks sometimes have to send requests (e.g. when responding to a 401 during
authentication).

All keyword arguments should be passed along when hooks are dispatched so that
if a user wanted to use a timeout, stream, specify a cert location with the
verify flag, etc, their specification can be followed.


The second commit adds a test which tests the previously broken behavior where flags like timeout, stream, verify, etc were lost when we used authentication. It fails without the patch.

if it's not clear, this tests by calling read() with stream turned on and off. If stream is off, then the content is read ahead of time by the adapter so further reads will return an empty string. If stream is on then the content will not be read ahead of time by the adapter, so the read in the test will receive the actual content. Without the patch the content is consumed by the adapter ahead of time regardless of the state of the stream flag.


This is going to break even the built-in authentication methods which I'm guessing are what's causing the tests to fail.


I already patched the built-in authentication methods. The tests are failing because in my new test response.raw.read() returns a str for me (python 2.7 and 2.6) and a bytes object on 3.x. This is a problem with my test, not the patch itself. I'll see if I can figure out a way to make the test py3k compatible.


FYI, If it would help get this accepted (it's fixing what I'd consider a serious bug) I'd be more than happy to contribute the necessary changes to requests-oauthlib, requests-ntlm, and requests-kerberos.


I'm installing python 3 because I haven't had to deal with byte arrays before.


That should do it, and it still works in 2.7.


come on travis, ...


BTW, I tested it under python-2.7, python-3.3, and pypy. If it doesn't pass this time I'll be surprised.


Did Travis completely miss that last commit?


Just trying to trigger Travis


Huzzah!


Does anyone have any suggestions as to how we can jump-start a conversation here? I know it changes the hook api, but it's necessary to actually do what the user requests.


Sorry this has been quiet, but Kenneth is the only person who merges PRs around here, so until he gets some time this won't move forward. He's crazy-busy (and unwell at the moment, I think), so we'll just have to wait. =)

For what it's worth, I'm +1 on this, hooks should have access to the full range of information.


Aha, I didn't expect a quick merge. I was just surprised there were no comments at all. Thanks for the info. I hope all is well.


I need to think about this. :)


I completely agree that this ensures what the user wants is done, it just makes me wonder if there's a better way of implementing it. We're passing all of that information around a couple times now.


Stick it on the session & pass that?

On Mon, Feb 25, 2013 at 9:35 AM, Ian Cordasco notifications@github.com
wrote:

> ## I completely agree that this ensures what the user wants is done, it just makes me wonder if there's a better way of implementing it. We're passing all of that information around a couple times now.
> 
> Reply to this email directly or view it on GitHub:
> https://github.com/kennethreitz/requests/pull/1190#issuecomment-14044761


They don't quite belong on the session. I would think they belong on the Request, but the request object describes the data we send, not the connection. What these do is describe the connection and response. I'll think about it some more


I'm liking this a lot.


:sparkles: :cake: :sparkles: