`Session.send` accepts arbitrary kwargs for the adapter, `.resolve_redirects` doesn't

In addition to its documented set of optional arguments (`stream=`, `verify=`, `cert=`, `proxies=`), `Session.send` accepts arbitrary keyword arguments, which are passed directly to the adapter's `send` method. (For instance, `HTTPAdapter.send` also accepts a `timeout` argument.)  `resolve_redirects` logically ought to do the same, but it does not: it only accepts stream/verify/cert/proxies.  This may, for instance, cause a timeout manually specified in a `send` call to get applied only to the first request and not to subsequent redirections.  If you are not manually walking redirections, you presumably want your `send` kwargs to get applied to all redirections in the chain; if you _are_ manually walking redirections, you may want to be able to control all of the adapter's supported `send` kwargs at each step.


Yeah, we should be passing all the keyword arguments through.


:+1: Nice find.


Just to clarify, if i change:
`def resolve_redirects(self, resp, req, stream=False, timeout=None, verify=True, cert=None, proxies=None):`
to:
`def resolve_redirects(self, resp, req, **kwargs):`
I need to add in the lines:

```
kwargs.setdefault('stream', self.stream)
kwargs.setdefault('timeout', self.timeout)
kwargs.setdefault('verify', self.verify)
kwargs.setdefault('cert', self.cert)
kwargs.setdefault('proxies', self.proxies)
```

and make sure to do:

```
allow_redirects = kwargs.pop('allow_redirects', True)
stream = kwargs.get('stream')
timeout = kwargs.get('timeout')
verify = kwargs.get('verify')
cert = kwargs.get('cert')
proxies = kwargs.get('proxies')

```

before i use those variables?

(I'm new to the project and am slightly unclear on the purpose of changing this, clarification would be great)


Hey @mikecool1000 welcome to the project. So you can't simply change the function signature like that, here's why.

Right now, someone could be doing this:

``` python
s = requests.Session()
# ...
s.resolve_redirects(resp, req, False, 1.0, True, None, {})
```

If we then change the signature, even one extra positional argument will fail, e.g., try the following:

``` python
def foo(a, b, c=3):
    return a, b, c

foo(1, 2)  # should return (1, 2, 3)
foo(1, 2, 4)  # should return (1, 2, 4)

def foo(a, b, **kwargs):
    c = kwargs.pop('c', 3)
    return a, b, c

foo(1, 2)  # should return (1, 2, 3)
foo(1, 2, 4)  # should raise a TypeError about foo only accepting 2 arguments and receiving 3
```

So you'd want to add `**kwargs` after the existing positional arguments (that have defaults). Maybe this will set you on the right course without giving you too much room to shoot yourself in the foot or giving you the all the answers. :)


Thanks that's very helpful

Sent from my iPhone

> On Oct 10, 2014, at 2:01 PM, Ian Cordasco notifications@github.com wrote:
> 
> Hey @mikecool1000 welcome to the project. So you can't simply change the function signature like that, here's why.
> 
> Right now, someone could be doing this:
> 
> s = requests.Session()
> 
> # ...
> 
> s.resolve_redirects(resp, req, False, 1.0, True, None, {})
> If we then change the signature, even one extra positional argument will fail, e.g., try the following:
> 
> def foo(a, b, c=3):
>     return a, b, c
> 
> foo(1, 2)  # should return (1, 2, 3)
> foo(1, 2, 4)  # should return (1, 2, 4)
> 
> def foo(a, b, **kwargs):
>     c = kwargs.pop('c', 3)
>     return a, b, c
> 
> foo(1, 2)  # should return (1, 2, 3)
> foo(1, 2, 4)  # should raise a TypeError about foo only accepting 2 arguments and receiving 3
> So you'd want to add **kwargs after the existing positional arguments (that have defaults). Maybe this will set you on the right course without giving you too much room to shoot yourself in the foot or giving you the all the answers. :)
> 
> —
> Reply to this email directly or view it on GitHub.


@mikecool1000 if you need other help, please hop onto #python-requests on Freenode this weekend. If I'm around, I'll be happy to chat with you about this.


Hey guys, while working on this issue, i found:

```
        # Redirect resolving generator.
        gen = self.resolve_redirects(r, request,
            stream=stream,
            timeout=timeout,
            verify=verify,
            cert=cert,
            proxies=proxies)

        # Resolve redirects if allowed.
        history = [resp for resp in gen] if allow_redirects else []
```

in the Session.send definition. I'm wondering if it would be better to not always call resolve redirects, and instead to choose to execute or not based upon whether or not allow_redirects is true. For example replace this code block with:

```
history = []
if allow_redirects:
     gen = self.resolve_redirects(r, request,
           stream=stream,
           timeout=timeout,
           verify=verify,
           cert=cert,
           proxies=proxies)
    history = [resp for resp in gen]
```

unless I'm mistaken(which is quite possible), this would avoid unnecessary computation


here is my first attempt at fixing this, no idea how good it is https://github.com/mikecool1000/requests/blob/fix-%231956/requests/sessions.py


@mikecool1000 how familiar are you with generators? You should try this out at the interpreter:

``` pycon
>>> import requests
>>> s = requests.session()
>>> r = s.get('https://httpbin.org/redirect/2', allow_redirects=False)
>>> g = s.resolve_redirects(r, r.request)
>>> type(g)
<type 'generator'>
```

When you run `g = s.resolve_redirects(r, r.request)` note that it will return immediately. It doesn't talk to the network until you start to consume the generator. There's no unnecessary computation at all :)


i figured it was something like that, but ive had no experience with generators in the past, sorry for being such a bad coder lol


No one called you a bad coder @mikecool1000 and you don't need to apologize for not having had prior experience with generators


Thanks for helping me learn then :)

Sent from my iPhone

> On Oct 13, 2014, at 11:07 AM, Ian Cordasco notifications@github.com wrote:
> 
> No one called you a bad coder @mikecool1000 and you don't need to apologize for not having had prior experience with generators
> 
> —
> Reply to this email directly or view it on GitHub.


Hullo! I see that `resolve_redirects` contains the `**adapter_kwargs` parameter already in `requests.sessions.py`:

```
 def resolve_redirects(self, resp, req, stream=False, timeout=None,
                          verify=True, cert=None, proxies=None, **adapter_kwargs):
```

So it looks like the arbitrary `kwargs` is getting passed through already in the function definition? Maybe I might be mis-understanding a thing. 


@onceuponatimeforever Ah, good catch, we just merged a fix for this a few days ago. =)