Raibek/requests-github-issues huggingface dataset:
Should Session.send persist cookies across calls?

I'm noticing that cookies are not preserved across calls to `Session.send` calls as they would be with `Session.request`. Once again I'm left with the desire for the ability to use my own `Request` object subclass in the scope of `Session.request`; e.g. if it were a key worded arg all my problems would be solved.

Below I've taken an example from the documentation and modified it to demonstrate that using `Session.send` directly eschews some nice functionality that you'd get "for free" using the `Session.request` method. However because I would like to make sure of my own `Request` subclass, without completely overloading `Session.request` on my own `Session` object or monkey patching Requests, I can't make use of `Session.request`.

``` python
>>> req = requests.Request(method='get', url='http://httpbin.org/cookies/set/sessioncookie/123456789')
>>> r = s.send(req.prepare())
>>> r.content
'{\n  "cookies": {\n    "sessioncookie": "123456789"\n  }\n}'
>>> req = requests.Request(method='get', url='http://httpbin.org/cookies')
>>> r = s.send(req.prepare())
>>> r.content
'{\n  "cookies": {}\n}'
```


@kennethreitz this seems sensible to me. I won't send a Pull Request though until you say so.


+1


I'll work on this tomorrow night or Thursday


Fixed by #1239
Ease painful migration to 1.x

At my company we have recently migrated to 1.1.0, and I have to say it's been much more painful than I expected. Finding what has changed since 0.x has been a nightmare. I have not been able to find good documentation on exactly what has changed, and the commit messages are atrocious. I don't see a clear way to move forward short of reviewing the diff of every single commit, and that's too time consuming.

Please add a CHANGES.txt file to the project and religiously document every single API change, especially the backward-incompatible ones. I love requests, it's much better than the alternatives. But the instability of the API and lack of documentation is making it hard to upgrade. Providing a path forward for existing users is a critical feature.


I'm sorry you found upgrading to v1.0.0 painful. You'll be pleased to know that as of v1.0.0 we have entered a [feature freeze](http://docs.python-requests.org/en/latest/dev/todo/#feature-freeze), which should cause the API to remain unchanged from now on.

As to having a CHANGES.txt file, I agree that we could better document the changes made in v1.0.0. At some point over the next couple of days I'll sit down and enumerate the whole set of changes. We may not add a CHANGES.txt file, but I'll document it somewhere, even if it's only on my blog.


Thank you! Sorry for coming across so harshly this morning. I was annoyed because I had spent quite a bit of time researching what had changed before upgrading (and came away feeling like I did not really know what had changed). Today I ran into what I thought was an API change that I had missed. When the smoke cleared I realized that I was getting confused between the Request/Response object APIs of Requests and WebOb.


It's not a problem at all, everyone's entitled to a bad mood now and then. =)


With #1231 merged, I'm closing this without prejudice (i.e., if we have to re-open it, I'm okay with that)
Please, do not hold development of requests with feature freeze.

I propose to make special branch `freeze` which would only get bug fixes and continue to develop requests in the master branch as always.


The intention is to bring development of new features in Requests to a halt and aim for inclusion in the standard library. This is fundamentally in opposition to the addition of new features, so it simply won't happen. Sorry.


@Lukasa 

What's wrong with designating one branch, where development of new features could be halted without hindering development in other branches?


HTTP is a nice and concise scope. The most important thing is to learn to say no.


+1.

We don't need to be libcURL for Python, and to be able to do everything. We need to be good at doing HTTP for 80% of the use cases. Anyone who needs the remaining 20% knows enough about HTML to be able to either extend the transport adapters/sessions or should use urllib2. In this sense Requests is a subscriber to the philosophy 'worse is better'.
Allow RequestsCookieJar to be updated with cookies from a CookieJar

This would allow cookies from external CookieJars, or other RequestsCookieJars to be merged in with a RequestsCookieJar while preserving all domain and other cookie info.


This would prevent calls like https://github.com/kennethreitz/requests/blob/master/requests/sessions.py#L158 (and the one on line 163) from losing domain information on the cookies, as well as let external CookieJars be merged into  a requests session's cookies cleanly.


This is definitely a must have. +10


I also just removed ability to do `requestscookiejar[somecookie]` this will not be needed if update supports CookieJars directly.

On another note, there may also be an issue with sessions merging cookies into requests: https://github.com/kennethreitz/requests/blob/master/requests/sessions.py#L254-L256 It appears that session cookies will overwrite cookies specified for the request right now (that have the same domain/path/key.)


thanks!
Throw more informative exceptions.

In #1188 @brandon-rhodes pointed out that we throw some uninformative exceptions when problems arise in urllib3. This would catch the exception and wrap it in a Requests exception, then rethrow it with its original traceback.

I'm on the fence about whether we should do one of the following:
1. Create a new Requests exception for this case (`requests.exceptions.TransportError`?)
2. Import urllib3's exceptions into `requests.exceptions` and export them as our own? (Not great)

Opinions welcome (@sigmavirus24?)


+10


So I just went ahead and did number 1.


Ugh, got some compatibility issues here. This will be problematic, give me some time to work on it.


So it turns out that there is no simple way to get around this Syntax Error. `six` has a good solution, but we currently don't vendor it in. The way I see it, we have three options:
1. Use the version of `six.py` in urllib3. This is bad: `six.py` is an implementation detail in urllib3, which is itself an implementation detail of Requests. -1
2. Add the bit of `six.py` that we need to our `compat.py`. This has a minimal effect on our code, but is a bit weird. +0
3. Vendor in a copy of `six.py`. This is the cleanest, but adds a whole new Python file from which we only need one line. Probably a small perf hit too. +0

@kennethreitz, @sigmavirus24: I don't know which is best from options 2 and 3. Thoughts?


I think we should leave things as-is for now.


In that case, we can't really do anything to improve these exceptions. =) Closing this.


I think being as transparent as possible is a good strategy — having the underlaying SSL errors bubble up, for example, is a great pattern to follow.


Oh, ok. Lemme rewrite this PR to do that instead. =)


Raise e :)


Pretty sure the blank `raise` simply rethrows the last caught exception, so should be fine. =)
Proxies without schemes might want to except.

:warning: **CAUTION**:  Breaking change. We may not want to take this. :warning:

This is in relation to #1182. Currently we guess what scheme to use on proxies if that scheme is not provided. This _usually_ works, but sometimes fails in unexpected ways. This would require users to be explicit about what scheme they want to use to contact the proxy.

This **will** break some users' code, without question. I am open to leaving the current behaviour in place, as it's not wrong, just a bit odd.


+0.5 I'm never for breaking user's old code in a minor version, but I am most certainly for providing better exceptions.


Exactly my view. =) I thought I'd open this PR so we can see what it entails, but I think we'll probably not fix what isn't broken. Just want to see what @kennethreitz thinks.


Considering my proxy code was already broken in one of the past few updates, I believe this exception would be very helpful in eliminating cryptic errors and letting the user know how to fix it. Any changes that users would need to make would be simple, anyways.


The following is a proposal for future releases:

Replace the parameter `proxies` of type dict with another one `proxy` which takes a string.
People in need of different proxies can use transportadapters after all.

I don't know (but assume) that deprecating the old parameter and introducing the new one isn't possible at this point.
This way old code won't break but new code would be cleaner. (When does one need different proxies after all)
This new api could then get rid of the guesswork.


@t-8ch: Unfortunately, changes to the API are not allowed in minor version bumps, and we're not going to go to v2.0.0 for quite a while. Additionally, I would argue that the [feature freeze](http://docs.python-requests.org/en/latest/dev/todo/#feature-freeze) means that the API is essentially frozen as well.


This was meant as a proposal for a future version, after the feature freeze.
I think the dict argument was a leftover from before transportadapters and is not needed anymore.
I also thinks it's very uncommon to use two different proxies (given the fact, that https proxies don't work at all at the moment, see shazow/urllib3#139)


@t-8ch: Requests' feature freeze is perpetual, there is no plan to exit the freeze. =) Your arguments are totally understandable, but the API is unlikely to be changed in any way from now on, and certainly not as major a change as that one.


FWIW, I believe that HTTPS proxies will work once fine once https://github.com/shazow/urllib3/pull/139 is merged. cc @t-8ch 


@wolever I don't doubt this! I only doubt the commonness of the need for two distinct proxies. Anyways I'll shut up now as I totally understand the decision.
Remove the line that likely causes the issue

Fixes #1189 as confirmed by @Lukasa


+1


+1


Actually, seems like this breaks persisting cookies across sessions:

Using the first example in [Session Objects Doc](http://docs.python-requests.org/en/latest/user/advanced/#session-objects):

``` python
import requests

s = requests.Session()

s.get('http://httpbin.org/cookies/set?sessioncookie=123456789')
r = s.get("http://httpbin.org/cookies")

print r.text
```

@ `77804d7`

```
{
  "cookies": {}
}
```


Yeah. I should have seen that on the test failure.


@nathankot thanks for catching this. Could you try with the latest commit added to this PR?


@sigmavirus24 looks good :)


:metal:


#1189 has been fixed in another way, so this pull should no longer be needed.
Multiple Cookies on Redirect

After updating `master` to f73bda06e9, I find that some requests raise a `CookieConflictError`. Specifically, any page that sets a cookie on a 302 and then redirects to a page that sets the same cookie raises a `CookieConflictError`.

Steps To Reproduce:
1. Update Requests to the version shown above.
2. Make a request to http://www.google.com/ from anywhere outside the US.

Expected Behaviour:
Requests should return a 200 OK with the 302 in the history.

Actual Behaviour:

``` python
>>> import requests
>>> r = requests.get('http://www.google.com/')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "requests/api.py", line 55, in get
    return request('get', url, **kwargs)
  File "requests/api.py", line 44, in request
    return session.request(method=method, url=url, **kwargs)
  File "requests/sessions.py", line 312, in request
    resp = self.send(prep, **send_kwargs)
  File "requests/sessions.py", line 426, in send
    history = [resp for resp in gen] if allow_redirects else []
  File "requests/sessions.py", line 163, in resolve_redirects
    resp.cookies.update(cookiejar)
  File "venv/lib/python2.7/_abcoll.py", line 494, in update
    self[key] = other[key]
  File "requests/cookies.py", line 246, in __getitem__
    return self._find_no_duplicates(name)
  File "requests/cookies.py", line 285, in _find_no_duplicates
    raise CookieConflictError('There are multiple cookies with name, %r' % (name))
requests.cookies.CookieConflictError: There are multiple cookies with name, 'NID'
```

This is very clearly related to the changes made in #1180.


/cc @sigmavirus24 


Damn. I was worried the last line might cause troubles but I couldn't find websites like this other than GitHub and didn't think to test it. (I thought GitHub may have been an abnormality.)

Can you try removing the last line of the `SessionRedirectMixin.resolve_redirect` method? I can't get Google to produce that error.


@Lukasa you can also try sigmavirus24/requests @fix1189


That works like a charm. =) PR time!


I should have turned back my paranoia ;).


I'm wondering if #1194 fixed this. I couldn't replicate the original issue though, so I'm not sure.
EDIT: Actually I'm fairly confident `_find_no_duplicates` should't be running anymore, so the cookie should just be overwritten now, which seems to be the proper course of action, and #1191 should no longer be needed.


I'll test this at some point soon.


@gazpachoking, you're quite right, this has been resolved. Thanks guys!
Fixed names of exceptions pertaining to url scheme. 

MissingSchema -> MissingScheme, InvalidSchema -> InvalidScheme
see t-8ch's comment in issue #1182


This is a backwards incompatible change.


@kennethreitz we could maintain backwards compatibility by keeping the old exceptions and sub-classing them for the new ones. It's ugly and I don't like it, but we would be able to raise "properly" named exceptions whilst maintaining backwards compatibility.


It looks like it's not only feature freeze but bug freeze as well :)


Werkzeug does the following to keep backwards compatibility when renaming:

```
class NewName(object):
    pass

OldName = NewName
```


@untitaker the only catch is that the error strings for these errors use "Schema" and should be changed to "Scheme" to be consistent, which is why I suggested sub-classing the originals.
requests.get doesn't allow for repeated URL parameters

If I prepare a request as follows:

```
req = requests.Request('get', url="http://localhost/", params=[("param1", "value1"), ("param1", "value2")])
prep = req.prepare()
```

prep.url is "http://localhost/?param1=value1&param1=value2" as expected. However if I call:

```
resp = requests.get(url="http://localhost/", params=[("param1", "value1"), ("param1", "value2")])
```

then resp.url is "http://localhost/?param1=value2"

I would expect requests.get to allow for repeated parameters.

(tested in version 1.1.0 and the revision f73bda0)


Hm, this is an issue with calling `merge_kwargs` in `Session.request`. This seems like yet another job for a MultiDict (#1155), i.e., returning a MultiDict in `from_key_val_list`.


Guess I'll be working on MultiDict support this weekend. I'm going to close this in deference to #1155 though, if that's ok with you @mattboehm 

**Note** The issue is not yet fixed.


Sounds great; I'll close this ticket. I was actually going to suggest using a MultiDict internally so I'm glad we're on the same page =).


:metal:


This issue is fixed.

```
>>> requests.Request('get', url="http://localhost/", params={'param1': ['value1', 'value2']})

```
Fix of UnicodeDecodeError on unicode header name that can be converted to ascii.

## Problem

Please see issue #1082.
## Solution

Let's fix at least Python-2 unicode issues with **header name**,
because its representation as bytes is well defined in HTTP/1.1 spec:
http://tools.ietf.org/html/rfc2616#section-4.2

```
message-header = field-name ":" [ field-value ]
field-name     = token
token          = 1*<any CHAR except CTLs or separators>
CHAR           = <any US-ASCII character (octets 0 - 127)>
```

So header name should always be sent as ascii.

If user provides header name that can be converted to ascii without errors,
then it should be converted, e.g. `u'Content-Type' --> 'Content-Type'`.

Otherwise a helpful error should be raised, indicating that the header name is the reason.
- Created: `test_unicode_header_name`.
- Updated: `prepare_headers`.


Why use a side-effect of the `str` function to do this, when you can easily do: `name.encode('ascii')`? Additionally, if you do that you can remove the 'if py2' tests.


@Lukasa
Thanks! Please check the fix.


I'm going to close and reopen to get Travis to re-run the tests.


This fix looks broadly right. My only concern is that this is a subset of the problem, but this is a perfectly good start to the problem. =)


One step at a time )


I don't recall off the top of my head where I've seen this, but I believe latin1 (or something similar) is recommended for header encodings. @mitsuhiko do you know?


@kennethreitz

`latin1` (aka ISO-8859-1) is recommended for header **values** by the very HTTP/1.1 spec
http://tools.ietf.org/html/rfc2616#section-4.2
- Header **name** is always `ascii`:

```
    message-header = field-name ":" [ field-value ]
    field-name     = token
    token          = 1*<any CHAR except CTLs or separators>
    CHAR           = <any US-ASCII character (octets 0 - 127)>
```
- Header **value** is `latin1` by default, but may be any other charset too:

```
    message-header = field-name ":" [ field-value ]
    field-value    = *( field-content | LWS )
    field-content  = <the OCTETs making up the field-value
                     and consisting of either *TEXT or combinations
                     of token, separators, and quoted-string>
    TEXT           = <any OCTET except CTLs, but including LWS>
    OCTET          = <any 8-bit sequence of data>

    Words of *TEXT MAY contain characters from character sets
    other than ISO-8859-1 (aka latin1)
    only when encoded according to the rules of RFC 2047.
```

This RFC is MIME for headers: http://tools.ietf.org/html/rfc2047

Example of encoded value:

```
    =?UTF-8?Q?=E2=98=91?=
```

It can be decoded with:
http://docs.python.org/2/library/email.header.html#email.header.decode_header

So it's a bit more complex than just `latin1`.

I guess it is a good idea to merge the fix for header **name** first,
and then return to header **value** issues.


beautiful, thanks :)
Attach a Certificate for SSL authentication and access.

Please add the ability to attach a Certificate for SSL authentication to the request, not to verify the cert, but to gain access to an https SSL secured site.

I am submitting this per Kenneth's request. 

Thanks, 
Brian


Isn't this, what the `cert` parameter is for?
From the [docs](http://docs.python-requests.org/en/latest/user/advanced/#ssl-cert-verification):

```
You can also specify a local cert to use as client side certificate, as a single file
(containing the private key and the certificate) or as a tuple of both file’s path:
```

``` python
>>> requests.get('https://kennethreitz.com', cert=('/path/server.crt', '/path/key'))
<Response [200]>
```


I finally figured out what was happening. Apparently the server I was trying to connect to only accepts TLS connections (not SSLv2 or SSLv3). I don't see anywhere in "requests" that allows you to change the ssl protocol used, or am I missing it? 

Thanks,
Brian

---

 From: Thomas Weißschuh notifications@github.com
To: kennethreitz/requests requests@noreply.github.com 
Cc: tigeryell99 tigeryell99@yahoo.com 
Sent: Sunday, February 10, 2013 1:20 PM
Subject: Re: [requests] Attach a Certificate for SSL authentication and access. (#1179)

Isn't this, what the cert parameter is for?
From the docs:
You can also specify a local cert to use as client side certificate, as a single file
(containing the private key and the certificate) or as a tuple of both file’s path: 

> > > requests.get('https://kennethreitz.com', cert=('/path/server.crt', '/path/key')) <Response [200]> 
> > > —
> > > Reply to this email directly or view it on GitHub..


Enjoy: http://lukasa.co.uk/2013/01/Choosing_SSL_Version_In_Requests/
improving `super_len`

when using `StringIO` in Python 3 oder using `cStringIO` in Python 2, you will have an object that does not have a `len` AND does not have a `.fileno()`

In that case, the best method for finding the length of the backing object is to seek to the end. This assumes that the object is seekable, though.

Since we're going to load the whole object into memory anyway, memory consumption should not be increased by the seeking.


I'd prefer us to do a little magic if possible. What's the use case here?


Use case:

```
# python 3.3.0
import requests
import io

requests.get('http://example.com/', data=io.StringIO('my data'))
```

Gives me:

```
Traceback (most recent call last):
  File "usecase.py", line 4, in <module>
    requests.get('http://example.com/', data=io.StringIO('my data'))
  File "/Users/spielmann/prog/tmp/requests/env/lib/python3.3/site-packages/requests/api.py", line 55, in get
    return request('get', url, **kwargs)
  File "/Users/spielmann/prog/tmp/requests/env/lib/python3.3/site-packages/requests/api.py", line 44, in request
    return session.request(method=method, url=url, **kwargs)
  File "/Users/spielmann/prog/tmp/requests/env/lib/python3.3/site-packages/requests/sessions.py", line 276, in request
    prep = req.prepare()
  File "/Users/spielmann/prog/tmp/requests/env/lib/python3.3/site-packages/requests/models.py", line 224, in prepare
    p.prepare_body(self.data, self.files)
  File "/Users/spielmann/prog/tmp/requests/env/lib/python3.3/site-packages/requests/models.py", line 349, in prepare_body
    length = super_len(data)
  File "/Users/spielmann/prog/tmp/requests/env/lib/python3.3/site-packages/requests/utils.py", line 49, in super_len
    return os.fstat(o.fileno()).st_size
io.UnsupportedOperation: fileno
```

In my use case, I am mirroring requests from one source to another using `io.StringIO`. Yes, I _could_ read the body and put it into a string. You could argue that it is never necessary to use `StringIO` as data, but you could argue the same for files, and `super_len` will actually go to the file system to read the length of a file.

I also think that the fileno call will fail when the file was deleted in the meantime but is still open in Python. I have not checked that, though. In any case, the argument is that it's not advisable to assume that we can read the file size from `len` _or_ from the filesystem.
Added the ability to track the referer between requests and cleaned up some docstrings

Added "track_referer" attribute and "last_page" to Session so that the last response's URL can be used as the referer on the next request. The header can still be overwritten globally via Session.headers or on per-request basis.


we aren't really interested in adding new features at this time.

the other changes looked pretty nice though, could you remove `track_referer` and `last_page` so I can merge?


I really need this to simulate the browser's behavior and makes my code pretty ugly to get the URL from the previous request and pass it to the next. How about if I generalize this to a hook that can modify headers and more right before the request is sent? This way, a lot more than just user agent tracking can be done. In general, hooks need to be expanded to all events (pre and post), but I'm sure you're well-aware of it. :)


@nikolay pre-request hooks were removed in the refactor. An (albeit ugly) way of doing this would be to use `allow_redirects=False` and manage it yourself, but as we're not accepting new [features](https://github.com/kennethreitz/requests/issues/1165), it is unlikely either hooks will come back or sending a referrer header will be accepted. The rest of your changes look great though. Thank you for them! :+1:


Okay then. I guess, my only option then is to create a separate module that wraps around Requests to simulate a browser's behavior more closely. I will then remove my changes and do more cleanups. :)


@nikolay would you rather I remove the referrer tracking or would you be willing to do it? Kenneth mentioned liking the rest of the changes and his desire to merge the pull for those changes. If not, just let me know.


I'm gonna do it later tonight - I just got busy with other things. There are a few more polishing I've done here and there.


Sounds good. I'm just checking up.


Closing for now — comment when we can reopen :)
Use send in resolve redirects

These three (which I'll happily squash if you'd like) should prevent the problem I mentioned [here](https://github.com/kennethreitz/requests/pull/1151#issuecomment-12905796).


+1
Generally speaking I've been thinking about raising issue with architecture for some time now :) The problem I see with current architecture is that redirect logic doesn't reuse standard path of sending requests and this causes all sorts of problems; I'm sure we've had more than few bugs because of this in the past. I'm glad you made the above changes.


I'm glad to have made you glad Piotr. It isn't something I seem to do too often. :-P


Hmm, looking at it more closely now... I think I might have missed the true nature of your changes. You propose to use `.send()` instead of `.request()` which actually is going in the opposite direction. Could you elaborate on your changes? Why do you say _we shouldn't be sending the data on redirect_?


Using `Session.send` is required. When we are managing everything (i.e., constructing, preparing and sending the request) we will ALWAYS have the original unprepared `Request` object. If instead, a user does the following (as mentioned in the comment linked above).

``` python
import requests

p = requests.Request('GET', 'https://httpbin.org/redirect/2').prepare()
s = requests.Session()
resp = s.send(p, allow_redirects=True)
```

Then `Session.resolve_redirects` will be relying on a non-existent `Request` object. The only way to make the API work for users is to either require (via a non-keyword argument) the unprepared `Request` or to more simply use the `PreparedRequest`.

As for not sending the data on redirect, if you look at the original call to `Session.request` upon redirect, the data was never sent. That's because it shouldn't since requests [behaves like a browser](https://github.com/kennethreitz/requests/pull/1044#issuecomment-11539872). I was simply making sure it behaved exactly as it was expected to prior to this change.


Ah, I remember why you're [concerned](https://github.com/kennethreitz/requests/issues/1084#issuecomment-12742129). I'll factor that in. Thanks.


Why is it necessary to have construct a `PreparedRequest` from scratch?


Rebased. And @kennethreitz I reconstruct a `PreparedRequest` because previously the original was what we made the `request` attribute on the `Response` object. This is just to keep backwards compatibility and not surprise anyone relying on that.
Fixed get_unicode_from_response() to do what it says in the docstring.

Also added a reasonable encoding in the fall-back case.


The docstring is incorrect then


Hm, why is that? To me it sounded like a reasonable heuristic.
Also, how come this function is nowhere used?


Because Requests is not an HTML library. =) We used to parse HTML, but now we don't. The function is left over from when we did.


Oh, I thought you were modifying the `get_encoding_from_headers` :)

This isn't used because requests is an http client, not an html client.


honestly we should likely remove this


but people might be using it


It's a good utility function. =)


I understand, but anyway, why isn't it simply called from response.text. Now there are 2 html functions.


Because it's incorrect to assume that data returned over HTTP is necessarily HTML, and even if it is, Requests shouldn't parse it.


Got it =)
Make test suite totally offline


We could use @kanzure's project which relies on HTTPretty or we could use mock like I do in github3.py. If you chose the latter, I'll be happy to work on the tests.


In particular, I think he is referring to [kanzure/python-requestions](https://github.com/kanzure/python-requestions). The httpetrified decorator is a simple way to use HTTPretty with a serialized Request or Response. However, it presently does not work with multiple requests or multiple responses.


Btw, my comment was not meant as a way of saying "Pick me because I'll do the work!", more of a, there are a couple different ways of doing it. I've helped @kanzure by just reading over kanzure/python-requestions a couple times and I like it, but I'm also quite familiar using mock directly. Testing redirects would be fairly ugly. If we were to do something like github3.py does it, we could have a method to craft the Response objects as the `return_value` on the mock and if we patched on the Adapter level, we would have to use mock's ability to have multiple return_values in succession. I have to re-read the docs to see how that's done though. Just an example of one of the slightly more challenging tests we might have to consider.


What about spawning a local HTTP(S) server like `urllib3`?


@schlamar if I remember correctly, this has been brought up before. Look up CherryPy or Twisted on the issue tracker. We could easily mock responses and test the like. It isn't that difficult. @kanzure and I do the same for our projects.


FYI, I'm working on [requests-vcr](/sigmavirus24/requests-vcr) which I think might make this a lot easier. Also it should make the tests a lot faster.


@sigmavirus24, well, while you're at it, please deprecate my requestions library. I made some poor decisions in its implementation that I think you can just copy. I know you're probably just copying the vcr gem from ruby, but maybe you can add in those decorators for mocking out tests etc.


It's going to take extra work to mimic the VCR gem from Ruby, but I'll look at how difficult it would be to add a decorator.

Currently, the usage I have in my head is this:

```
def setUp(self):
     self.session = requests.Session()
     self.session.headers.update(...)

def test_user_endpoint(self):
    with VCR(self.session).use_cassette('user') as vcr:
        resp = self.session.get('https://api.github.com/user', auth=('foo', 'bar'))
        assert resp.json().get('login') is not None
```

With perhaps some exposition of some mock methods, e.g., `assert_called_with`, etc.

The call to VCR will be necessary so that we do not have to monkey-patch any libraries (yay transport adapters!).


Now that Betamax exists and seems to cover the majority of use cases (including gzip-encoded response bodies) should we start using that for integration style tests and some combination of unit and acceptance tests?


Closing in deference to https://github.com/kennethreitz/requests/issues/2184
Problem when using PUT in v1.1

Hi:

  I'm having a problem when making PUT requests. This problem is not present on version 0.14.2, but it has arisen when I updated the module.

  It seems that the content of the PUT (data), which should be encoded as a POST, is not being properly encoded. At least, I am not able to receive it when checking on the server. It appears as empty. If I try the 'params' parameter, then it is properly encoded as a QueryParam.

  The curious thing is that using a POST seems to work fine. I assume that the interface is still the same.


HTTPBin works fine for me with 'PUTs', can you provide some of the code you're using so I can try to reproduce the issue?


Ups, my mistake. I've been looking at that more carefully, and it turns out to be a problem on redirections...

The call was being redirected, so the body wasn't being sent again (which is what it should happen). Our framework was doing other stuff, that was raising a different error when finding an empty body...

My mistake, not Requests ;-)


No worries, thanks for coming back and updating us. =)
What happened to danger_mode?

I was just going to suggest this feature when I noticed pull request https://github.com/kennethreitz/requests/pull/333.

Looking in the current code, I can't find any indication of `danger_mode`, or a similar setting. Was there a reason this was removed? I'd be happy to contribute to bringing it back.


I suspect it got pulled out during the big refactor. I'm not opposed to having it back, depends what @kennethreitz thinks.


Danger mode is now the default and the only mode. Safe mode was regarded as useless. Now, that's good news for you! :)


I'm pretty sure it was removed because it was deemed an unnecessary configuration option. And yes, it was deliberately removed during the refactor.


@piotr-dobrogost is correct, it is the default. @zwass, are you okay with closing this issue?


In which version? I'm using 1.1.0 and I get this:

``` python
>>> import requests
>>> requests.__version__
'1.1.0'
>>> requests.get("http://www.google.com/fdsljfd")
<Response [404]>
>>> 
```


@zwass you want to use `raise_for_status`. That was never the purpose of `danger_mode`. `danger_mode` simply suppressed underlying exceptions during the connection or cert validation, basically anything that sub-classed `requests.exceptions.RequestException`. If you look at the end of the page [here](http://docs.python-requests.org/en/v0.14.2/user/quickstart/?highlight=safe_mode) then you might understand it better.


@sigmavirus24 according to that documentation (and the pull request), `danger_mode` caused `raise_for_status` to be called at the end of any request. If this were the default behavior in 1.1.0, my example above would have thrown an `HTTPError: 404 Client Error: Not Found`.

It seems that the default behavior in 1.1.0 is `safe_mode`, but there is no ability to use the equivalent of `danger_mode`.


Hm, the docs didn't indicate that. The problem is that the entire Configuration API was deliberately removed in the refactor. As such, there isn't really a `safe_mode` either, just the in-between that always existed. A request is a request and any exception that occurs during that is raised. I doubt @kennethreitz would like to re-add this (given there is likely not a very elegant way of doing so without adding the Configuration API back), but I'll re-open it to be fair.


I won't re-add it, however, we need to document this changes better and the intentions behind these decisions

Also, roadmap.

In general, we aren't interested in adding new features anymore. This needs to be clear.


Understood about the configuration API. Perhaps we can add a `fail_fast` keyword argument to `requests.request` that would cause `raise_for_status` to be called after the request is executed.


That's equivalent to re-adding part of the configuration API. It's also a new 
feature and as @kennethreitz mentioned, we aren't adding new features.


Does that mean that requests might merge into the Python standard library soon?


Once things settle down, yes :)


Sweet :+1: 

Well, I guess I'll resign myself to calling `raise_for_status` after every request.


#1165
Proxy Authorization Headers

This is one half of the pair of open problems from #1053. Please code review this before pulling it.

I concluded that, as this is fundamentally an authentication issue, this belongs in our code, not in urllib3. For the moment, I have it set up so that if the authentication is provided in the URL, we simply grab it out. It might be worth turning this into a separate part of the API though. Let me know what you think.


Goddamn it Travis! =D


Travis is the child we love when it does what we want and hate when it 
doesn't... except it isn't our child.


currently, proxies are only supported in the connection adapter layer, so this shouldn't be surfaced higher up

as always, correct me if i'm wrong :)


Not sure. I'll sleep on it and see how I feel.


in general, i want us to support proxies in a very very light manner. I hate them.


Mm, you're probably right =). I'll take another look at it tomorrow evening and see if I can implement it at that layer in a nice way.


Shiny new commit with all the logic in the transport adapter.

I added an `add_headers` function to the `HTTPAdapter` because I suspect most future transport adapters will heavily rely on this one. Adding specific headers is useful logic, and this allows the adapter to be easily subclassed and have new headers added: only one function override is needed.


<3
If Content-Length is already set, don't override

Re: #1159


Nice. =)

It should be noted that this will not resolve the issue raised in #1159, as if I set Content Length to anything falsy this will fail (this includes zero). I don't know if we want to allow the forcing of Content-Length to weird values this way, but if we do you'll need to take another look at this.


Well all headers should be strings. Any non-empty string will work. One could 
set: `{'Content-Length': 'fail'}` and it should work as far as we're 
concerned. Setting `{'Content-Length': ''}` is valid as well, but do we want 
to support it?


Depends if we want to use this method to prevent the header appearing entirely or not.


Typically if the user didn't want a header to appear, they would use 
`{'Header-key': None}`, but you're right, I'm not sure how we'd swing that. On 
the other hand, we wouldn't be following spec if we didn't include it when 
there is a body (assuming that is an instance where the user doesn't want to 
send it).


I'm generally in favour of requiring that users go out of their way to do stuff that will almost certainly break servers.


So we're in agreement ;)

I guess the one change we should make is to ensure that Content-Length's value 
is a string, but really if the user is mucking with it, then they should have 
the decency to look up how it's supposed to work. I swear, if we get any 
issues due to this (e.g., why won't the Content-Length header accept ints?), 
I'm going to click close so hard :-P.


Does this use the case-insensitive dict?


@kennethreitz, PreparedRequests don't use the CaseInsensitiveDict currently. If you would like it to, let me know.
Auth handlers are unable to modify query strings/form data sanely

I'm trying to port the parameter-based authentication scheme that AWS uses to requests 1 from an older version.  It uses a request's parameters to generate signature parameters of its own and then augments the request with them.  The resulting parameters then go into the request as either a query string or form data.  As a result, this handler needs to be able to modify those, but since requests 1 just hands auth handlers PreparedRequests and no references at all to the original Requests, query strings and form data (and in fact everything other than headers) become incredibly inconvenient to modify.

I'm handing the auth handler to a Session and then calling that Session's request method a bunch of times.  Is there a way to make that (also?) give the auth handler the original Request whenever it gives it a PreparedRequest without breaking an interface somewhere?  Should I be unpacking everything with urlparse and cgi and then re-calling prepare_url and prepare_body?  Or is there another fix/workaround that might be better?


Basically, you should just parse those things out of the body. Fairly simple.


Yeah, it's just more obnoxious than modifying the dict that produced them since it involves unpacking and repacking the body.  :-\  Do I have any hope of getting the info from the original Request to the auth handler as well?
Fix check that breaks handling of 303 response in some cases

This fixes issue #1156.

Signed-off-by: Yehuda Sadeh yehuda@inktank.com


Thanks for the fix! From a personal perspective, I'd rather you changed the line to:

``` python
if resp.status_code == codes.see_other and req.method != 'HEAD':
```

Both versions (yours and mine) will fail if `codes.see_other` ever returns more than one code, but I'm not keen on creating a single-element tuple just to walk it and do the comparison. We may as well just do it.
Added r.ppjson() functionality.

I find myself continuously making a pretty_print type method when dealing with r.json() commands. Figured it may better placed back inside requests.


I don't really see the need for this to be in Requests. Requests is an HTTP library. We only have a `json()` method because it's a insanely common use-case and it's convenient. Printing the JSON string in a nice way is a less common case and nothing to do with HTTP. =)

I just don't think this is worth cluttering the API for.


I whole-heartedly agree with @Lukasa


Correct. Basically, we're not really interested in new features anymore. We should communicate this in the documentation.

Thanks for the contribution, though! :cake:


Good points. Thanks for all the awesomeness that still is "requests"! :)
use only base filename in multipart file upload

I have a tiny patch, which enforces the correct behavior in multipart/form-data file uploads. The function guess_filename in requests/utils.py extracts the filename from the file-like-object from which we stream the file's contents. In the case of a file, this gives the full path on the local filesystem to the given file, which is then propagated to the request headers like this (in urllib3/filepost.py):

'Content-Disposition: form-data; name="%s"; filename="%s"\r\n' % (fieldname, filename)

RFC 2388 says this:

> The original local file name may be supplied as well, either as a
> "filename" parameter either of the "content-disposition: form-data"
> header or, in the case of multiple files, in a "content-disposition:
> file" header of the subpart.

It's my understanding that the correct behavior, implemented in web browsers, is to include the file's _name_ and not its full _path_ on the local filesystem. I added a call to _os.path.basename_ to shorted any paths to the base filename.


seems good to me!
Move redirect handling from Session.request to Session.send

This closes #1133 and allows users to use Session.send with a prepared request. The tests pass, but I just thought of a separate instance where this may not work properly, so don't merge right away.


Yeah, my intuition was correct:

``` pycon
>>> import requests
>>> r = requests.Request('GET', 'https://httpbin.org/redirect/2')
>>> p = r.prepare()
>>> s = requests.Session()
>>> resp = s.send(p)      
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "requests/sessions.py", line 394, in send
    history = [resp for resp in gen] if allow_redirects else []
  File "requests/sessions.py", line 96, in resolve_redirects
    method = req.method
AttributeError: 'NoneType' object has no attribute 'method'
```

@kennethreitz are you okay with req (or the Request object) being mandatory as a parameter, e.g., `Session.send`'s signature becomes: `def send(prepared_request, request, **kwargs):`?


ping @kennethreitz, thoughts on changing `Session.send`'s signature?


I'll review as soon as I get a chance :)


Ah carp. I included a different patch in with this. (The fix for the `elapsed` attribute's test on 2.6)


:+1:


:sparkles: :cake: :sparkles:
On some pages requests detect encoding incorrectly

For example it detects http://www.15min.lt encoding as ISO-8859-1, but the encoding is set as utf8 <meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />


This is a duplicate of #1087. You can see there that we don't read the content to determine the encoding. If you want you can use `apparent_encoding()` to get an encoding from [charade](/sigmavirus24/charade). You can also use `from requests.utils import get_encodings_from_content`, but we don't parse the HTML anymore. We simply report encoding from the Headers and if there is none, we default to ISO-8859-1.


To clarify @sigmavirus24's point, we only default to ISO-8859-1 in [some cases](http://docs.python-requests.org/en/latest/user/advanced/#encodings).
Update cacert.pem

Updated cacert.pem with latest from http://curl.haxx.se/ca/cacert.pem


so, this cert list is great, however it contains a bunch of blacklisted certs as well.

I've been manually removing them. They need to be removed again. We should maintain a list perhaps.


Nod nod. Perhaps on the wiki?


why remove them?  to keep the size down?


@ansel1, why remove blacklisted certs? Because otherwise, urllib3/ssl will assume we want to trust those certs when we shouldn't.


python's ssl doesn't support blacklisted certs.


i see.  where do you get the list of blacklisted certs?  Or how do you distinguish them in cacerts.pem?  I'm trying to prune my own copy to get something working today.  


unfortunately, we have to remove them by hand at the moment. We should automate this. I'm not sure what is involved exactly at this point.


I would like to remove them by hand from my own copy, but I don't know how
to identify them.  How do you know which ones to remove manually?

On Tue, Feb 5, 2013 at 12:30 PM, Kenneth Reitz notifications@github.comwrote:

> unfortunately, we have to remove them by hand at the moment. We should
> automate this. I'm not sure what is involved exactly at this point.
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/kennethreitz/requests/pull/1149#issuecomment-13141114.


a contributor did it before, i think there's a seperate blacklist in the mozilla repo. 


closing for now.
It`s more pep8 now


I disagree with one of these changes.

We'll see how long I can stand it :)


Demanding one blank line (instead of the two) between function definitions is a very bad idea. We should try to create some coding standard for Requests.


@piotr-dobrogost why is it a bad idea?


@sigmavirus24 

Because one blank line is a good way of separating blocks of code inside a function. In light of this separating functions with only one blank line doesn't separate them enough.


I've always felt like the nice yellow word 'def' and the change in indentation made it pretty clear. =)


@piotr-dobrogost I guess it is merely a matter of preference. I use PEP8 for all my projects. I've never had a problem determining between two methods. And two functions (not methods) are always separated by 2 blank lines as per pep8.


@piotr-dobrogost calm down
Bad Request on some sites

``` python
print requests.get('http://thepiratebay.se/recent').content
```

the code above runs normally in requests 0.14.2
but, tpb returns bad request in requests 1.1.0


This is hard for me to test [in the UK](http://www.guardian.co.uk/technology/2012/apr/30/british-isps-block-pirate-bay), but my magic mind powers suggest that this might be related to #1051, which was fixed by #1142. I don't think that fix is in PyPI yet, but can you try installing the current repository version in a virtualenv and see if it works?


This is what I see pre #1142

``` python
>>> import requests
>>> r = requests.get('http://thepiratebay.se/recent')
>>> r
<Response [400]>
>>> dir(r)
['__bool__', '__class__', '__delattr__', '__dict__', '__doc__', '__format__', '__getattribute__', '__hash__', '__init__', '__module__', '__new__', '__nonzero__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', '__weakref__', '_content', '_content_consumed', 'apparent_encoding', 'close', 'connection', 'content', 'cookies', 'encoding', 'headers', 'history', 'iter_content', 'iter_lines', 'json', 'links', 'ok', 'raise_for_status', 'raw', 'reason', 'request', 'status_code', 'text', 'url']
>>> r.content
'<?xml version="1.0" encoding="iso-8859-1"?>\n<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"\n         "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">\n<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">\n <head>\n  <title>400 - Bad Request</title>\n </head>\n <body>\n  <h1>400 - Bad Request</h1>\n </body>\n</html>\n'
>>> r.history
[]
```

And after #1142 

``` python
>>> import requests
>>> r = requests.get('http://thepiratebay.se/recent')
>>> r
<Response [200]>
```

So @Lukasa is correct, and this is a duplicate.