Improve API for manual redirection-following

I'd like to propose a bunch of bugfixes and API improvements to redirection resolution, particularly when it's being done manually (i.e. `allow_redirects=False` on the initial `send()`).  These are all motivated by problems I encountered while trying to chase redirects for every single URL in the Alexa top million, which, as you might imagine, contains an awful lot of misconfiguredness (up to and including IMAP and SMTP servers on ports 80 and 443!)

The most significant change is the new `Session.resolve_one_redirect` method, which does what it says - it resolves _one_ redirect.  This turns out to be substantially more convenient for applications that need to do complicated processing on each redirect as it happens, than the existing `Session.resolve_redirects` generator.  It goes along with `Response.is_redirect`, a new property that is the canonical home for the "is this a redirect" predicate.

The second most significant change is that each response in a redirection chain now has an accurate `.history` property, containing all responses up to but not including itself.  As a side effect, I anti-resolved issue #1898 - `.history` is now always a _list_.

The third most significant change is that `resolve_one_redirect` and `resolve_redirects` do not need to be passed a bunch of arguments that were already passed to the initial `send`; concretely, all of `send`'s kwargs are cached on the `PreparedRequest` and reused thereafter.

Everything else is bugfixes, generally in the service of greater robustness against Weird Shit coming off the network.


Wow, this is a substantial change, and an impressive bit of work. I'm not going to dive into code review at this moment for two reasons: I don't have time, and I think we'll want @kennethreitz involved early in the discussion on this pull request.

Nevertheless, there'll definitely be a couple of code review comments if we decide to move ahead with this. I'm mostly interested to see what Kenneth has to say about the idea though.


I haven't looked into this at all, but I'm curious as to why you cannot just 
call `next(self.resolve_redirects())` once instead of having to write a 
special method for that. One more reminder: I haven't looked at any of this.


@sigmavirus24 re `next(self.resolve_redirects())`, the most significant reason for doing it differently is that `resolve_redirects` doesn't include the very first response in its iterable (and I didn't feel safe changing that).  That means code that needs to look at each response as it comes in is most naturally structured like this:

```
r = sess.send(..., allow_redirects = False)
while r.is_redirect:
    # ... do stuff with r ...
    r = sess.resolve_one_redirect(r)

# ... do more stuff with r, which is now the final response ...
```

rather than like this:

```
r = sess.send(..., allow_redirects = False)
# ... do stuff with r ...
for r1 in sess.resolve_redirects(r):
    # ... do stuff with r1 ...

# ... do more stuff with r? or r1? blech ...
```

which duplicates part of the processing and has some finicky logic after the loop to get hold of the final response.


I'm getting a 503 Service Unavailable when I try to see the results of the failed CI build.  All tests did pass in my local environment, but it's entirely possible that I was not running them correctly (not having found any documentation on how to run them, I was just doing `PYTHONPATH=... ./test_requests.py`)


> Nevertheless, there'll definitely be a couple of code review comments if we decide to move ahead with this. I'm mostly interested to see what Kenneth has to say about the idea though.

I feel confident that Kenneth will be -1 as I am, but because of the size of the PR I've already started leaving code review comments in the event I'm wrong.

That said, there are several changes here that conflict with the explicit feature freeze put into place before 1.x which continues to be upheld.


It occurs to me that it would be helpful if I shared the code that motivated most of these changes.  The entire program is here: https://github.com/zackw/tbbscraper/blob/master/sources/canonize  A great deal of it is uninteresting for this conversation; the code that does most of the interacting with Requests is [the `HTTPWorker` class](https://github.com/zackw/tbbscraper/blob/master/sources/canonize#L621), and I direct your attention specifically to [`process_one_response`](https://github.com/zackw/tbbscraper/blob/master/sources/canonize#L658), especially the bit where it says "This logic must match requests.session's idea of what a redirect is", and to [`chase_redirects`](https://github.com/zackw/tbbscraper/blob/master/sources/canonize#L681), which has the core loop-over-redirections in it.


@zackw Also, as you reply to PR feedback (since there will be so much of it here) can you reply to each comment your fixing with "Fixed in <sha>". I won't mind the emails sent and it will help us keep track of what was fixed and when. It also provides more context when performing final reviews.


> can you reply to each comment your fixing with "Fixed in "

Can do.


> Can do.

Thank you.


FYI going to bed at this point, will respond to further comments tomorrow.


I've glanced through the code review comments stuff so far, and wanted to talk more generally about how this relates to our API freeze.

@zackw has said in one of his comments that he believes these changes would be welcome extensions to the API for anyone who has to manually handle redirects. That's probably true. However, our API freeze policy does not say that we are freezing the API "except when it'll be useful for people if it was extended". By default, the Requests answer to API changes will always be "no". The reason this issue is still open and being discussed is because we think there is potentially enough value here that we want to look at it in depth. Please don't assume that we hate your PR, @zackw, we're just starting from a very conservative position.

Next, there is a question about how much affordance we should give to people who circumvent Requests' redirection policy. The general Requests policy on this sort of thing (see also: `PreparedRequest` objects) is that if you don't like the way Requests does it you should do it yourself. We have made concessions here in the past, but not many and always under substantial duress.

Again, I'm going to hold off more dramatic code review until @kennethreitz gives an idea of whether he's likely to want this change at all.


@zackw can you explain the `Response.is_redirect` reasoning to me? I can understand the thought process, as redirects are a very first-class citizen in the HTTP world, but I'd love to hear your feelings on it :)


Thank you _so much_ for this major contribution to the project, by the way! These look like incredible improvements to an underutilized API. I can tell you put some great care and craftsmanship into this :)

---

For context — Requests is a very large project and we care a lot about the code that gets in, so we have be very selective about new features. So, let's refine what we have here and see what feels right for both of us :)


I think I'm +1 for `Response.is_redirect`. Seems great. :sparkles: :cake: :sparkles:


The changes to the history implementation also sound wonderful, based on your description. If they behave as described, I believe I'm +1. They'll need some heavy code review, however. There's a lot of session and socket leaking that can happen with that code if we're not careful.


I gotta get other work done this afternoon, but, would it be useful for me to split this pull request into two? One strictly for bug fixes, and another for anything that changes the API even a little.

@kennethreitz given that you are open to the possibility of some amount of API changes, I'd like to observe that one API change I didn't make, but rather wanted to, was to have `Session.resolve_redirects` return an iterable that _does_ include the very first response.  Such a generator (under another name, for compatibility's sake) would be a workable, perhaps even preferable, alternative to the `resolve_one_redirect` API I did add.


@zackw make one for `Response.is_redirect` first. I'll merge it right away :)


@zackw perhaps we could give resolve redirects an argument to include the first response? 


e.g. `Session.resolve_redirects(r, full=True)` or something similar. 


(close/open for ci build)


@zackw let's break each change into a new PR as we discuss them and keep this one open for discussion. Then, we can bite one thing off at a time. 

So, first thing first — open a new PR for `Request.is_redirect` :)


Failure with 2.6 only.  Well, at least that reassures me it isn't my inability to run the test suite correctly :smile:  (I can only conveniently test locally with 2.7 and 3.3.)

2.6's `urlparse` apparently doesn't throw an exception when asked to parse `http://example.[^com/]`; it cheerfully treats that as valid, and we get a ConnectionError instead because, unsurprisingly, there is no top-level domain named "[%5Ecom".  I will think about what the abstract right thing would be.


+1 on using smaller PRs.

Also @zackw I'm sorry that none of us ever answered you: we use `py.test` to run the tests.


Alright, made some review comments inline. They are a supplement to @sigmavirus24's. =)