Raibek/requests-github-issues huggingface dataset:
KeyError during register_hook

If an event is added that isn't "response", during req.prepare() a KeyError is thrown.

The error is difficult to track down and relates to the copying process of hooks from the request to the PreparedRequest.

There should either be a better error message displayed. I.e. hook event not supported

Or, as is in my pull request ( #1524 ) the RequestHooksMixin.register_hook method should handle creating the event.


This is definitely a less-than-ideal behaviour, thanks for reporting this!

It seems to me that the right thing to do here is to throw a `ValueError` from [`register_hook`](https://github.com/kennethreitz/requests/blob/master/requests/models.py#L141) if a hook we don't support is added. =)


This I can get behind. We could also potentially raise a NotImplementedError but that is idiomatic to methods on classes that are meant to be overloaded by child classes. A ValueError is the next best choice. 


I'm happy to provide a patch so that others aren't going to hit this issue.

I would prefer a ValueError as stating that it's not implemented suggests that it's going to be. Which given the argument you've provided sounds unlikely.


We'd be delighted to have a patch, thanks! =)


In fact now only you can submit a patch for this issue. You're our only hope.


This will be resolved by #1527. Thanks so much!
Fixed KeyError on non deafult HOOK

At present, if you attempt to attach a hook that isn't for the event 'response' you'll get a KeyError during initialization of PreparedRequest.

I intend to use the hook mechanism to add logic of my own and therefore create my own events.

I believe this would be useful functionality for anyone extending the requests framework.


We only support the 'response' event so we do not need to handle this. If you'd like to extend the current system of hooks you'll probably have to subclass quite a number of objects to make them dispatch your hooks when you intend. We have no way of knowing when you want us to dispatch your hook if we're not explicitly supporting it.

tl;dr I'm -1 on this.


Mm, I'm broadly with @sigmavirus24. Seeing as you'll need to monkeypatch _anyway_ to get any of your hooks to be called, it's not a huge chore to monkeypatch this method to do the right thing. If we were going to take this, we'd have to decide to take a whole 'custom hooks' package, and that just seems like massive unnecessary complexity.

Thanks for the pull request though, and please do keep contributing!


But I don't monkey patch, I extend the session object to add events.

What's the point in adding a hooks system if it breaks when you try to use it. I'm simply requesting a fix of the code that already exists.


I think this comes from a misunderstanding of the role of the hooks. They were never intended as extensible functionality. If you look at [the hooks file before we went to 1.0](https://github.com/kennethreitz/requests/blob/v0.14.2/requests/hooks.py), you'll notice that we explicitly enumerated the hooks and limited which hooks you could actually have. During the move to 1.0, we removed all the hooks but `response`. This was for two reasons. Firstly, we had excellent anecdotal evidence that they were essentially unused. Secondly, they were replaced with the vastly superior `Request` + `PreparedRequest` combination. This provided nearly unlimited access to the data through the whole sending process.

If you're extending the session object, you can simply extend the hooks mixin and load your new one instead of the old one. Exactly the same modification style, but requires no additional complexity upstream. =)


> Firstly, we had excellent anecdotal evidence that they were essentially unused.

What was it?


@piotr-dobrogost No-one ever talked about them. Not in blog posts, not to Kenneth, not in GitHub issues, no handlers used them. We also got, as far as I recall, no complaints when we removed them. =)


> as far as I recall, no complaints when we removed them. =)

We received less than a handful but more than 0.
Add content_type property to response

Header `response.headers['Content-Type']` can include charset and other stuff. Consider to add `response.content_type` property with content type only.

Rationale. With this property such check will be allowed: `response.content_type.enswith(('/json', '+json'))`


Thanks for raising this issue!

What's wrong with:

``` python
'json' in response.headers['Content-Type']
```


```
What's wrong with:
```

This is not durable. This allows `json` in any place within `Content-Type` header. Any way, this is just example. `content_type` without stuff could be useful in many cases.

``` python
response.content_type in ('text/html', 'text/xml', 'application/xml')
```


So it's worth noting that the default response of this project to any request to change the API is No. An API change request starts with -100 points and needs to justify itself to that level before inclusion.

Sadly, I don't think this is sufficiently useful to justify inclusion. It would get a decent amount of use, but it's not hard to do yourself and not super common (like JSON decoding). I just don't think the API would benefit particularly from its inclusion.

I'll wait until @sigmavirus24 and/or @kennethreitz comments on this, but I'm -1. Sorry. =(

NB: The transformation for anyone that needs it is

``` python
ct = response.headers.get('Content-Type', '').split(';')[0]
```

Any RFC-2616 compliant Content-Type header should come through that just fine, including no header or an empty header.


I'm with @Lukasa. This is convenient but non-obvious frankly. Most people will expect this to return the entire header not just one portion of what was returned. Even if this had a chance of making it I would want to rename it but there unfortunately isn't a better name.
Requires python to be run as root (sudo) on Ubuntu LTS

```
$git clone git://github.com/kennethreitz/requests.git
$cd requests/
$python 
>>> import requests
URL [method [body | -f filename]]

Does a TCP connect and HTTP transfer, defaults to GET
$
```

falls over back to the command line, but

```
$sudo python
>>> import requests
>>>print requests.get('https://github.com/timeline.json').text
```

works ok

I'm running Python

```
Python 2.7.3 (default, Apr 10 2013, 06:20:15) 
[GCC 4.6.3] on linux2
```

On 

```
Ubuntu LTS 12.04.2 LTS
```

Which is 

```
Linux 3.5.0-37-generic, x86_64
```

Any assistance will be greatly appreciated.


Works as expected on a Red Hat. (Linux 2.6.32-20130307.60.9.bh6.x86_64)


Breaks on

```
Ubuntu 12.04.1 LTS
```

Which runs

```
Linux laptop 3.2.0-48-generic-pae, i386
```


Does it break on different machines running 12.04.1? Does it brake for other users on the same machine?


Breaks on both the 12.04.1 and 12.04.2 machines that I have.
It works for other users.


The message you are seeing is totally stumping me.
Could you search your filesystem, for `Does a TCP connect and HTTP transfer` this is not from requests, but probably python tries to import something it shouldn't.


And can you play with the `-E`, `-s` and `-S` command line switches to the python interpreter.
Could you also post the output of running the code while suppling `-v` to the python interpreter?


I only searched /usr/lib/python2.7. It is loading a custom http.py script on the $PYTHONPATH rather than the intended internal one. Thanks.
Requests get fails when env proxy does not have prefix

I am running requests release 1.2.3 and had problem that I tracked down to the following:

$ export http_proxy=some-proxy.this.that
$ python

>   import requests
>   requests.get('http://github.com')

gets:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.6/site-packages/requests-1.2.3-py2.6.egg/requests/api.py", line 55, in get
    return request('get', url, *_kwargs)
  File "/usr/lib/python2.6/site-packages/requests-1.2.3-py2.6.egg/requests/api.py", line 44, in request
    return session.request(method=method, url=url, *_kwargs)
  File "/usr/lib/python2.6/site-packages/requests-1.2.3-py2.6.egg/requests/sessions.py", line 335, in request
    resp = self.send(prep, *_send_kwargs)
  File "/usr/lib/python2.6/site-packages/requests-1.2.3-py2.6.egg/requests/sessions.py", line 438, in send
    r = adapter.send(request, *_kwargs)
  File "/usr/lib/python2.6/site-packages/requests-1.2.3-py2.6.egg/requests/adapters.py", line 272, in send
    conn = self.get_connection(request.url, proxies)
  File "/usr/lib/python2.6/site-packages/requests-1.2.3-py2.6.egg/requests/adapters.py", line 197, in get_connection
    conn = ProxyManager(self.poolmanager.connection_from_url(proxy))
  File "/usr/lib/python2.6/site-packages/requests-1.2.3-py2.6.egg/requests/packages/urllib3/poolmanager.py", line 123, in connection_from_url
    return self.connection_from_host(u.host, port=u.port, scheme=u.scheme)
  File "/usr/lib/python2.6/site-packages/requests-1.2.3-py2.6.egg/requests/packages/urllib3/poolmanager.py", line 109, in connection_from_host
    pool = self._new_pool(scheme, host, port)
  File "/usr/lib/python2.6/site-packages/requests-1.2.3-py2.6.egg/requests/packages/urllib3/poolmanager.py", line 72, in _new_pool
    pool_cls = pool_classes_by_scheme[scheme]
KeyError: 'some-proxy.this.that'

This seems to be related to Issue #997 
If 
$ export http_proxy=http://some-proxy.this.that

all works fine.

Sorry if I missing anything obvious. 

Julius
(requests is awesome!)


Thanks for raising this! This should be fixed in the next major release of Requests by #1497. =D


Thanks!
requests does always use .netrc credentials ignoring the ones specified at login

It took me two full days of work to discover that python-requests was using the credentials from the `~/.netrc` file and ignoring the ones that I was specifing.

I think that the correct behaviour should be to use the specified ones and ignore the netrc ones. Even the documentations seems to agree with me. Still, this is not what I discovered.


Should be fixed in 2.0.
See #1438/#1439


Thanks! Any idea when we will see this release it c


Totally up to Kenneth, I'm afraid. It'll be shipped when he's happy with what we've got. =)
Why doesn't the Response constructor accept arguments?

Reading through the code in `adapters.py`, I notice that a Response is created, and then has values assigned to it, like this:

```
    response = Response()

    # Fallback to None if there's no status_code, for whatever reason.
    response.status_code = getattr(resp, 'status', None)

    # Make headers case-insensitive.
    response.headers = CaseInsensitiveDict(getattr(resp, 'headers', {}))

    # Set encoding.
    response.encoding = get_encoding_from_headers(response.headers)
    response.raw = resp
    response.reason = response.raw.reason
```

Is there a reason that these values are not passed directly into the Response constructor?

I ask because I'd like to use the Response class for passing around HTTP response data internally (for things that aren't necessarily immediately following a `requests.request`, and the constructor feels like a logical place to do variable assignment.


In addition it looks like `requests` only initializes a Response object in one place and a refactoring could be fairly simple


There's no reason beyond style. We prefer it that way. =) You'll notice that the `Session` object has exactly the same interface.
[2.0] Https proxy support


Looks like these test failures are transient, I think this is good. Thanks so much for your work @schlamar, as well as all of the urllib3 people who worked on this! =)


Yeah once HTTPbin is pushed to heroku, these too will pass.


[+1](http://youtu.be/qybUFnY7Y8w).


+1


socks5 proxy support


@yfengz This will probably take some time, see https://github.com/shazow/urllib3/pull/68


:sparkles: :cake: :sparkles:
Started receiving intermittent ClosedPoolError's now that my AppEngine app is multi-threaded

> > ClosedPoolError: HTTPSConnectionPool(host=OMITTED, port=443): Pool is closed.

Given that the requests homepage says requests is thread-safe, how should I handle these errors? Should I catch this exception and simply re-try, or is this a bug with the requests library?


This could arise from your usage but in general we don't support AppEngine. If you can provide some code samples I might be able to suggest a different strategy but AppEngine could be the cause of your problems. Were I you, I'd investigate Rackspace if possible 


Or heroku, of course :)
HTTPS proxy support

HTTPS proxy support finally made it into urllib3 (https://github.com/shazow/urllib3/pull/170), this is a patch for requests to support it.


Finally! I spent 4 hours playing with openssl and different combinations of requests itself and different forked urllib3's... I'm happy it looks everything is working fine now.


This is a great PR. =) One suggestion: it might be better to raise it against the 2.0 branch.


> This is a great PR. =) One suggestion: it might be better to raise it against the 2.0 branch.

x2


Is there an ETA for 2.0 release?


Not too far in the future. AFAIK we're not planning to have another point release before then. =)


> Not too far in the future. AFAIK we're not planning to have another point release before then. =)

So why is there a separate branch at all? :)


However, here we go: https://github.com/kennethreitz/requests/pull/1515
valueerror when unquoting bad url

Maybe it should return another error?

```
>>> requests.get("http://%zz")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/requests/api.py", line 55, in get
    return request('get', url, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/requests/api.py", line 44, in request
    return session.request(method=method, url=url, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/requests/sessions.py", line 324, in request
    prep = req.prepare()
  File "/usr/local/lib/python2.7/dist-packages/requests/models.py", line 222, in prepare
    p.prepare_url(self.url, self.params)
  File "/usr/local/lib/python2.7/dist-packages/requests/models.py", line 333, in prepare_url
    url = requote_uri(urlunparse([scheme, netloc, path, None, query, fragment]))
  File "/usr/local/lib/python2.7/dist-packages/requests/utils.py", line 383, in requote_uri
    return quote(unquote_unreserved(uri), safe="!#$%&'()*+,/:;=?@[]~")
  File "/usr/local/lib/python2.7/dist-packages/requests/utils.py", line 364, in unquote_unreserved
    c = chr(int(h, 16))
ValueError: invalid literal for int() with base 16: 'zz'
```


Thanks for raising this! We should be handling it as part of 2.0: see #1514. =)


Thanks for your pull request Lukasa.
I am still having a problem. How could I achieve same behavior as the browsers for the redirect below?
Thanks again for your help!

http://pixel.everesttech.net/1824/cq?ev_sid=3&ev_ln=assistance%20%C3%A0%20distance&ev_crx=6814336969&ev_mt=b&ev_n=g&ev_ltx=&ev_pl=&url=https%3A//secure.logmeinrescue.com/welcome/default.aspx%3Fdestination%3D/FR/welcome/remotesupport/default.aspx%26wt.srch%3D1%26utpk%3Dassistance%20%C3%A0%20distance%26originid%3D16808

returns an percent-escape exception when browsers redirects to

https://secure.logmeinrescue.com/welcome/remotesupport/?wt.srch=1&utpk=assistance+%u00e0+distance&originid=16808&ef_id=UbIrlwAABS4E32%403%3a20130810145257%3as


Yeah, that's an invalid URL in the second case: %u00e0 is not a valid string in a URI.
No option to turn off SSL compression

I could be missing something but it looks like there is no way to turn off SSL compression.


You're quite right, there is not. In no small part this is because this functionality is not exposed (at least not cleanly) in versions of Python before 3.3: additionally, AFAIK, urllib3 does not expose the functionality (@t-8ch knows the TLS support of urllib3 way better than I do, he'll be able to tell you).


Nope urllib3 doesn't expose this at the moment.
It doesn't seem the `SSLContext.options` expose really much functionality at the moment.
I think your best bet would be to monkeypatch `urllib3.util.ssl_wrap_socket()` (when used with requests it is `requests.packages.urllib3.util.ssl_wrap_socket()`) to set the options.

However if you think it is a common use case feel free to open a pull request over at urllib3.
It should be fairly easy to add this. Just look for `ssl_version` to find the locations where you should make your changes.
It would probably be best if the version of `ssl_wrap_socket` not using `SSLContext` throws a `ValueError` if options are passed in.


Thanks for the quick responses!

One further question: I'm a bit new to dynamic languages' idea of monkeypatching. Would that be a change in Requests or would I be monkeypatching from my own code which uses Requests?


You would monkeypatch from your own code that uses Requests. =)


Thanks :smile: 


Quick example, I simply copied the function from urllib3 and added the one line:

``` py
import requests
from ssl import SSLError, SSLContext, HAS_SNI, OP_NO_COMPRESSION


# copied from urllib3/util.py (for the greatest part)
def ssl_wrap_socket(sock, keyfile=None, certfile=None, cert_reqs=None,
                    ca_certs=None, server_hostname=None,
                    ssl_version=None):
    """
    All arguments except `server_hostname` have the same meaning as for
    :func:`ssl.wrap_socket`

    :param server_hostname:
        Hostname of the expected certificate
    """
    context = SSLContext(ssl_version)
    context.verify_mode = cert_reqs

    # !!!! Only change
    context.options = OP_NO_COMPRESSION

    if ca_certs:
        try:
            context.load_verify_locations(ca_certs)
        # Py32 raises IOError
        # Py33 raises FileNotFoundError
        except Exception as e:  # Reraise as SSLError
            raise SSLError(e)
    if certfile:
        # FIXME: This block needs a test.
        context.load_cert_chain(certfile, keyfile)
    if HAS_SNI:  # Platform-specific: OpenSSL with enabled SNI
        return context.wrap_socket(sock, server_hostname=server_hostname)
    return context.wrap_socket(sock)

# monkeypatch
requests.packages.urllib3.connectionpool.ssl_wrap_socket = ssl_wrap_socket

print(requests.get('https://httpbin.org/'))
```
stream with io.ByteIO

I can't use io.ByteIO in order to "put" a stream of data

In fact Requests use 'super_len' in "prepare_body" in order to check its length, and "super_len" checks for the "fileno" attribute.
 io.ByteIO has that attribute, but it "Returns underlying file descriptor if one exists.An IOError is raised if the IO object does not use a file descriptor."

So I always get a "UnsupportedOperation: fileno" error.

I see that super_len is under a "try:.. exception (TypeError, AttributeError):" ; I propose to add the "io.UnsupportedOperation" exception among the others


Before we do that, why are you using `io.ByteIO`? Can I see a sample of your code?


Beh, really I'm not using directly io.ByteIO 

I'm uploading a file on a webserver running werkzeug, who get the file in a FileStorage structure; the I proxy the content directly to a couchdb storage.

FileStorage uses io.ByteIO

I can avoid the problem by fixing my source code, but probably other modules/softwares use io.ByteIO for streaming and the "io" library is among the standard libraries (at least in 2.7), so I proposed this patch.


Aha, that makes sense. Yeah, I'm in favour of using file-like objects here, so we should probably catch this exception. +1.


I'm also +1 for this.


This should hopefully be resolved in 2.0: see #1516. =) Thanks for suggesting this!
AttributeError: 'MockRequest' object has no attribute 'host'

This issue appears to be specific to Python 3 (with requests 1.2.3), and I believe it only occurs with cookies that specify port numbers.  It looks to me like the "MockRequest" object defined in cookies.py provides a get_host() method that would work here, but the code inside cookiejar.py tries to go directly to the host attribute, bypassing the mock.  Below is a stack trace showing the issue:

Python 3.3.1 (v3.3.1:d9893d13c628, Apr  6 2013, 20:30:21) [MSC v.1600 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.

> > > import requests
> > > request = requests.head('https://cloudsquid.redewt.net:2083/')
> > > Traceback (most recent call last):
> > >   File "<stdin>", line 1, in <module>
> > >   File "C:\Python33\lib\site-packages\requests\api.py", line 77, in head
> > >     return request('head', url, *_kwargs)
> > >   File "C:\Python33\lib\site-packages\requests\api.py", line 44, in request
> > >     return session.request(method=method, url=url, *_kwargs)
> > >   File "C:\Python33\lib\site-packages\requests\sessions.py", line 335, in request
> > >     resp = self.send(prep, *_send_kwargs)
> > >   File "C:\Python33\lib\site-packages\requests\sessions.py", line 438, in send
> > >     r = adapter.send(request, *_kwargs)
> > >   File "C:\Python33\lib\site-packages\requests\adapters.py", line 337, in send
> > >     r = self.build_response(request, resp)
> > >   File "C:\Python33\lib\site-packages\requests\adapters.py", line 176, in build_response
> > >     extract_cookies_to_jar(response.cookies, req, resp)
> > >   File "C:\Python33\lib\site-packages\requests\cookies.py", line 109, in extract_cookies_to_jar
> > >     jar.extract_cookies(res, req)
> > >   File "C:\Python33\lib\http\cookiejar.py", line 1647, in extract_cookies
> > >     if self._policy.set_ok(cookie, request):
> > >   File "C:\Python33\lib\http\cookiejar.py", line 931, in set_ok
> > >     if not fn(cookie, request):
> > >   File "C:\Python33\lib\http\cookiejar.py", line 1044, in set_ok_port
> > >     req_port = request_port(request)
> > >   File "C:\Python33\lib\http\cookiejar.py", line 628, in request_port
> > >     host = request.host
> > > AttributeError: 'MockRequest' object has no attribute 'host'


What version of requests are you using? I feel like we fixed that already.


Nope, I can hit it on Python 3, Requests 1.2.3. =)


You may be recalling https://github.com/kennethreitz/requests/issues/1369 which I came across while looking into this one.  Different issue though.


There's a really easy fix here, in cookies.py add this to the mock

``` python
@property
def host(self):
    return self.get_host()
```

That fixes the problem on my machine. Who wants to open the PR?
Update urllib3 to d89d508

This fixes a bug in urllib3 that caused the Host header to be set incorrectly when a proxy is used together with a nonstandard port number.

For example, when going to http://example.com:8000/ through a proxy, the Host header was set to 'example.com' instead of 'example.com:8000' which totally breaks Requests in such a case.


Looks reasonable to me, there doesn't seem to be any problematic API changes.
Reset hooks after registering them?

In `models.py` line 200, hooks is registered, which is actually copying `hooks` to `self.hooks`:

``` python
self.hooks = default_hooks()
for (k, v) in list(hooks.items()):
     self.register_hook(event=k, hook=v)
```

Then in line 212, `self.hooks` is reset again:

``` python
self.hooks = hooks
```


Good catch. Fixing this now.


Thanks again for filing this
[2.0] Proxy urls should have explicit schemes.

This is for 2.0, and is in response to @t-8ch's suggestion that proxies should have explicit schemes instead of guessing (which is stupid).

@sigmavirus24, @kennethreitz: I'm not entirely happy with the validation code being here: let me know if you have a better idea for where it should go.


I recently checked was curl is doing with different proxy schemes.
Curl ignores the scheme and always connects to the proxy using plain http.
What curl does support, what could be nice to have are `CONNECT` tunnels for plain http, if the proxy is messing things up.


Shorter stacktraces == easier to read exceptions for most users. If we can do the validation when someone sets `Session.proxies` or passes the `proxies` parameter that would be awesome. The lowest they would then see would be `Session.request` which isn't all too horrible all things considered.


@sigmavirus24 Yeah, I thought about that. Unfortunately, users can send directly into a Transport Adapter (it's a supported flow), and if they did that then they would lose this validation.


Ah good call. This is an obvious :+1: then.


Closing since this is merged into #1509
Can't install requests-1.2.3 on Python 3.3.2 via pip

edit: sorry, I just realized Lukasa already responded to this [here](https://github.com/kennethreitz/requests/issues/1416#issuecomment-21607795)

This is [issue 1416](https://github.com/kennethreitz/requests/issues/1416) again, I'm new to github and don't know how to reopen it or what, so I figured I'd do this.

I was able to install requests by downloading the zip and doing setup.py install just fine.

Anyway, it yields this:

```
C:\Python33\Scripts>pip install requests
Downloading/unpacking requests
  Downloading requests-1.2.3.tar.gz (348kB): 348kB downloaded
  Running setup.py egg_info for package requests

Installing collected packages: requests
  Running setup.py install for requests

      File "C:\Python33\Lib\site-packages\requests\packages\urllib3\contrib\ntlm
pool.py", line 38
        """
         ^
    SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes in pos
ition 130-131: truncated \uXXXX escape

Successfully installed requests
Cleaning up...
```

I mean, it says successfully installed but uh, it also throws an error.


@tz18 For those approaching this issue in the future, please note that the fix is currently in the repository, it just hasn't been pushed to PyPI. Feel free to check out the GitHub code instead if you think you'll use NTLM.
add context manager support to Response

This addresses the issue of exceeding the max number of open files.
This provides a simple, elegant way to make sure connections are closed after use.

Add **enter** and **exit** to Response.

e.g.
    with requests.get("someurl") as r:
         ... # do something with r

And alternative would be to provide a configuration for requests to automatically close connections after the response is created and contents retrieved. This would be useful in application where all responses are known to be short and chunking retrieval will not be used.


Thanks for raising this issue @julie777!

I'm strictly +0 on this. I can see the utility of it, but I don't know that a context manager on `Response` is semantically the right way to approach this issue. If @kennethreitz is +1 on this, though, I think it'd make a good addition to V2.0.


I fully understand the motivation but the context of a request doesn't quite 
semantically make sense to me.

The context of a session is intuitive, but not that of a response. Consider me 
-1 on this with that non-explanation.


I actually want to revisit this issue.

A not-uncommon use case of Requests is to open connections to download _part_ of a response, without downloading the whole body. In that case, to ensure that the connection pooling logic of `Session` objects still works, you need to make sure you call `Response.close()`. In this sort of use-case it does actually seem not-unreasonable to turn `Response` objects into context managers. @sigmavirus24, thoughts?


For this not-unvery-such-rare case, [`contextlib.closing`](http://docs.python.org/2/library/contextlib.html#contextlib.closing) might sufice. Though it doesn't hurt to implement this, right?


@pepijndevos It's not very hard to make objects context managers. (Not to sound like a pompous ass who's done this a million times)

@Lukasa I think I know what you're talking about -- there have been a fair number of bug reports/stackoverflow questions about this. I'm willing (because people seem to find `Response#close` annoying) to bend on this. I still think that logically it's a bit of nonsense but from a purely utilitarian perspective, it makes sense.


@pepijndevos I totally missed what you meant (contextlib provides a lot of things to help _make_ context managers). I didn't realize that it would close the thing. On that topic I'm 100% for documenting that pattern instead of making the `Response` object a context manager unto itself.
Session CookieJar

Suggested solution for #1488

Allows for code such as:

``` python
#!/usr/bin/env python
import requests
import cookielib
c = cookielib.LWPCookieJar(filename="test.cookies")
s = requests.Session(cookiejar=c)
r = s.get("http://www.reprap.no/")
c.save()
```


Hi @kefir-, thanks for the Pull Request!

Unfortunately, I don't think this will be accepted. As mentioned in #1488, you can set the `Session.cookies` parameter to an arbitrary cookiejar without difficulty. That is the expected way to use a different CookieJar type.


Thanks for the details.
Save cookiejar to file

Hi,

requests uses a RequestsCookieJar object internally, which is the object type returned from cookiejar_from_dict(). Am I missing a way to specify a different type of CookieJar object?

I want to save cookies back to file after an HTTP session as done by LWPCookieJar. The way I've done this so far is to traverse s.cookies and add the cookies manually to my LWPCookieJar, as shown in the following code. I think it would be nicer if I could tell Session() what type of CookieJar to use, so that I could save directly afterwards.

``` python
import requests
import cookielib

s = requests.Session()
r = s.get("http://www.reprap.no/")

jar = cookielib.LWPCookieJar(filename="test.cookies")
for c in s.cookies:
    jar.set_cookie(c)
jar.save()
```

Something like this looks like it should work with fewer lines of code, but I'm not sure this is the best approach in case something were to change in future versions:

``` python
s = requests.Session()
s.cookies = cookielib.LWPCookieJar(filename="test.cookies")
r = s.get("http://www.reprap.no/")
s.cookies.save()
```


This already works. Seriously, try your example in the second line of code, it works fine.


I was a little unclear on what you mean by "second line of code", but from your comment on my pull request I think you mean the second and shorter way to do this. Doing it that way works fine, like you say, so I'm happy to close this issue and my pull request.

``` python
s = requests.Session()
s.cookies = cookielib.LWPCookieJar(filename="test.cookies")
```


By the way, `Session().cookies` could perhaps be mentioned here in case someone else is looking for the same in the future:

http://www.python-requests.org/en/latest/api/#request-sessions


That's a good idea, I'll fix that up. =)


Changes made in #1490. Thanks so much for raising this issue! :cookie:
Rewrite test cases to remove dependency on httpbin.org and example.com

Partial solution to #1485

Only fixes the uses of httpbin.org; there are still a couple of test cases referencing example.com

I'm not sure if this solution is too much of a hack...


I don't think this is a hack at all, I think this is an excellent refactor/improvement of this test. =) +1 


Cool :)

Then I'll try to do the same kind of thing for the remaining ones that are hard-coded.


It's worth noting that the title of this PR is misleading, though: this is not a solution to running offline. =)


Offline is probably the wrong word.  My intention is to make it possible to run the tests without having to connect to an external site.


Title updated :)


I don't think `test_uppercase_scheme` is redundant, it exercises some top-level code that a redirect doesn't.


It's redundant because the upper case HTTP is tested in the mixed case
scheme test that I refactored in one of the previous commits.

It's not related to the redirect test case fix.


Ah, yes, you're quite right. =)


OK, so with this series of commits it's possible to run the tests completely against a locally running httpbin server.


Yes. It does. Thank you @dpursehouse
Use urlparse.urljointo construct httpbin url

Just a slightly more reliable way of joining the url parts.


This won't work in Py3. =) Probably worth grabbing this out of the `compat` file. =)


sigh :)


Fixed.=P


I might make a single-use website: http://www.willitworkonpython2and3.com/

Its markup:

``` html
<html>
<body>
    <h1>NO</h1>
</body>
</html>
```


@Lukasa love it :)


:+1:
Not possible to use local httpbin server for all unit tests

The test script allows to define `HTTPBIN_URL` in the environment to specify a locally running instance of httpbin to be used for testing.

However there is one test case (`test_mixed_case_scheme_acceptable`) where the url is hard-coded to httpbin.org, and two test cases (`test_uppercase_scheme` and `test_uppercase_scheme_redirect`) where the url is hard-coded to example.com.

This prevents testing of the Requests modules while behind a proxy that blocks outgoing requests (in my case specifically the https requests) or on a system without external internet connectivity (such as an internal CI system).


See issue #1166


I'm not sure how fast #1166 is progressing. It's tentatively earmarked for 2.0, but it might not make it.


Sorry, I hadn't seen #1166 
My bad.

As far as I can see, fixing this one unit test should at least make it possible to use the current test suite offline.


No worries. =) I'm not sure this fits our definition of 'offline' though: having to download and run httpbin is a bit of a hassle.


> having to download and run httpbin is a bit of a hassle.

Specially when you're behind a proxy that doesn't like `pip` and you have to manually download and run setup.py for each of its dependencies, and their dependencies :(


> Sorry, I hadn't seen #1166 

No need to be sorry. I'm trying to develop a solution for #1166 right now. If it comes down to the point where I don't have it ready for 2.0 I'm just going to use mock (which is in the standard library now as of Python 3.3) to make everything work offline. This would make a lot of things a bit easier.
Add Eclipse and PyDev files to .gitignore


These files are specific to your environment and should not be in project's gitignore but in your global gitignore file.


They are not in my global .gitignore file because I work on some projects where they are actually needed to be kept under version control.


I'm not for merging this at all.


Yeah, I think rather than do this you should put these settings into your project's `.git/info/exclude` file, which acts like a non-tracked `.gitignore` file.
Test cases fail when `HTTPBIN_URL` does not have trailing slash

Test cases can be run against a local httpbin server defined by
the `HTTPBIN_URL` environment variable, but it causes tests to
fail if the given URL does not end with a slash.

Ensure that the URL always ends with a slash.


Do you mind putting a comment here just saying why this is being done, so we don't forget down the line?


No prob.  I just amended the commit to include a comment.


Cool, I'm +1 on this. Ready to merge. =D


There's a cleaner solution to this using a urllib function but I'm not sure of the name at the moment.
Fix test cases that fail when running behind a proxy

When sending a request via `Session.send()` the proxies must be
explicitly given with the `proxies` argument.  This is not done
in the test cases, which means that they fail when run on a system
that is behind a proxy.

Add the `proxies` argument in the calls to `Session.send()` in test
cases.


Rather than use this on each call to `Session.send()`, can you set `Session.proxies` at the top of the routine?


Do you mean in `test_mixed_case_scheme_acceptable`?

It wouldn't reduce the diff because a new Session object is created before each call to send().

So instead of several blocks like:

```
s = requests.Session()
r = requests.Request('GET', 'http://httpbin.org/get')
r = s.send(r.prepare(), proxies=proxies)
self.assertEqual(r.status_code,200)
```

We end up with several blocks like:

```
s = requests.Session()
s.proxies = proxies
r = requests.Request('GET', 'http://httpbin.org/get')
r = s.send(r.prepare())
self.assertEqual(r.status_code,200)
```


A new session is only created the first two times. =)


OK.  I've amended the commit to make it set the proxies with `s.proxies = getproxies()`


Cool, I'm +1 on this. =) Thanks so much!
[2.0] Add copy method to PreparedRequest objects

I really think it is only needed for `PreparedRequest` objects, but I could see a desire to implement it on `Request` objects too.

The reasoning is simple. Currently, we have duplicated code to make copies of `PreparedRequests` to preserve the history of the requests sent and be able to make new ones. We could not only reduce duplication, but also ensure (via tests) that the library won't break in weird ways because of this in the future. It would be a dead-simple method to write, and would be very useful and probably even more useful to authentication handler authors.


For reference, here's [one way of implementing this](https://github.com/sigmavirus24/requests/commit/b84547d78699a3a5c75dbdf2c2e422dbaa83b988).


I think there's no good reason not to go straight to a Pull Request with that commit. =)


Kind of wish I could just turn this issue into a PR but I don't have the privileges here. Also we should make that 2.0 branch so I can target that since you cannot edit a PR's target after it has been created. 


Actually, @Lukasa, you can do that for me with this issue. :-P


Don't you have the same privileges as me with this repository?


As of the last time I checked, no. I'll try again though and edit this comment with the results.

Clearly I was wrong. :)


:sparkles: :+1: :sparkles:


:heart: :hamburger: :haircut: 


@kennethreitz is this good for merging?


Also, do you want tests for this? 


oh thought i did
[WIP] [2.0] Clean up Exceptions

This PR is a work in progress. It addresses some problems we've got with Exceptions. Currently it contains two fixes:
- Resolves the misspelling of 'scheme', reimplementing the fix proposed in #1187.
- Attempts to provide a fix of #1294.

This second part is of particular interest. I want to know what @sigmavirus24 thinks of this possible fix.


Let include the old misspelled version too, for compatiblity's sake. Simple subclass :)


Done. =)


I think that if we're making breaking API changes, we shouldn't be concerning ourselves too seriously with backwards compatible error changes. The point of the semantic version bump is that things are going to change in a manner that is backwards incompatible.


@sigmavirus24 i agree, but it's so easy, might as well. It's one line :)


No worries. Just playing devil's advocate. :)


quick rebase against the `2.0` branch?


Done. =)


This is failing because someone (:wink:) still hasn't merged [these changes](https://github.com/kennethreitz/httpbin/pull/110) on httpbin.


My name is Kenneth and I receive too many pull requests.

Best problem ever :)


Its time to scale. Introduce Kenneth robot to handle pull request?
On Aug 2, 2013 7:34 AM, "Kenneth Reitz" notifications@github.com wrote:

> My name is Kenneth and I receive too many pull requests.
> 
> Best problem ever :)
> 
> —
> Reply to this email directly or view it on GitHubhttps://github.com/kennethreitz/requests/pull/1475#issuecomment-21982443
> .


@kennethreitz indeed the best problem ever. Fwiw I'd be happy to help with httpbin's PRs.

Also @kracekumar we can rebuild him faster, stronger, better. ;)


Uh, @kennethreitz, I think you may have merged #1509 into this instead of the other way around...

**EDIT**: No, I'm just an idiot.


lukasa, there's a ton of stuff in here now. What's the status?


Yeah, took me a while to work out what happened. The PR is still against master, but I rebased against 2.0 at your request. If you'd prefer I can try to reopen against 2.0.
Collect cookies sent on a 401 Challenge

On 401's the cookies received aren't sent back to the server. Fixes: #1336

Also this will rely on some changes in HTTPBin for testing. I'm going to start a pull over there too.


And to elaborate. One of the tests will fail until the changes I added over on kennethreitz/httpbin#110 are accepted. Although those are not 100% correct (i.e. I don't think we should always be expecting a Cookie header to be returned. Then again, if this works, we should be fine with using that. Tests will just fail until then.


As a side note, we tend to make copies of PreparedRequests in at least two places (now) so I'm all for adding a `.copy()` method to them to make it easier to duplicate them. Objections?


@Lukasa some eyes on this early on will be helpful too.


This is still failing because we need the changes in httpbin first, but in the meantime I'd love to refactor this after #1476 gets accepted.

Otherwise this should fix everything related to the bug linked above.


Rebased against master to for 286ddb6.

Any new feedback would be more than welcome.


quick rebase?


@kennethreitz it's failing on travis because there's a related PR on HTTPbin that needs to be merged for the tests to pass. This doesn't need to be rebased at the moment, there's no merge conflicts according to GitHub.


oh sorry


BTW, we're getting sponsorship from rackspace, so we should have a real jenkins cluster soon :)


Can't wait to move off of travis.


Safe to assume this also fixes them for 301's?


Closing since merged into #1509


> Safe to assume this also fixes them for 301's?

No. I wasn't aware there was an issue with those. I'll hop on that too. Can you add a failing test?


> BTW, we're getting sponsorship from rackspace, so we should have a real jenkins cluster soon :)

Woot woot!


Oh there may not be one. I thought there was.


Cookies are collected on redirects successfully.

I think @gazpachoking worked on that successfully. This was a similar problem 
that wasn't able to be handled the same way.


most excellent! 

:dango: 


:metal:
DecodeError in 1.2.3; regression from 1.2.0

```
...
/project/virtualenv/lib/python2.6/requests/packages/urllib3/response.py", line 188, in read
    "failed to decode it." % content_encoding)
DecodeError: Received response with content-encoding: gzip, but failed to decode it.
```

This is against a URL which 1.2.0 handles without a problem. Since 1.2.3 this exception pops up.


I'm prepared to believe this was broken by commit 6d6252aa9fc4bb38d7f68073b41092c31ddb146b, and more specifically [this commit](https://github.com/shazow/urllib3/commit/d00d3052382de69382e21af8d10524e5aefadde8). 

I'm pretty sure this failure is in urllib3, not Requests: can you open this issue over there, and cc @schlamar on it?


Sounds reasonable. I'll open up an issue over there. Thanks!
"AttributeError: 'NoneType' object has no attribute '__dict__'" when using custom authentication class

A short script to reproduce this issue is here:

https://gist.github.com/dpursehouse/6039233

At this point I'm not sure if it's a bug, or I'm missing something obvious and just doing it wrong...


Doing something wrong. Your output looks like it's from a version of Requests pre-v1.0.0. =)


It's from a version installed a couple of weeks ago.   And I get the same
on a different computer where I just installed the latest version
yesterday...


Oh I'm sorry, I'm being an idiot! =D Though to be fair, the traceback on a newer version of Requests is totally different. =P

The `__call__` method has to return the updated Request:

``` python
import requests
class TestAuth(requests.auth.AuthBase):
    def __call__(self, r):
        return r
```


Ah, ok.  Thanks, I'll try that later.


It works as described.  Documentation is updated, so this can be closed now.
Remove charset from JSON types: not valid.

This resolves #1467.


Did github change their responses?


Oh, I was doing this off the back of httpbin's headers, which I thought was what we were using at this stage of the docs. Haven't checked GH.


ah, gotcha. indeed, incorrect.


I've been thinking more about allowing json uploads lately. would be pretty cool.


Could make it part of 2.0. =)


+1!


I'm thinking:

``` python
url = 'http://httpbin.org
data = {'some': 'data'}

requests.post('http://httpbin.org/post', data={'some': 'data'}, json=True)
```
Included charade package is licensed under the GPL

https://github.com/kennethreitz/requests/blob/master/requests/packages/charade/__init__.py

IANAL but doesn't this make requests unusable in commercial products? The top-level Apache license is a bit misleading.


It's under the LGPL (as far as I can see). Given that we haven't modified `charade` in any way, and that the user can easily swap `charade` for their own copy, I think this means we're fine. We probably _should_ provide the `charade` and `urllib3` licenses in a top-level file though.


We used to...


https://github.com/kennethreitz/requests/blob/master/NOTICE

## 

Kenneth Reitz

On Jul 19, 2013, at 7:28 AM, Cory Benfield notifications@github.com wrote:

It's under the LGPL (as far as I can see). Given that we haven't modified
charade in any way, and that the user can easily swap charade for their own
copy, I think this means we're fine. We probably _should_ provide the
charade and urllib3 licenses in a top-level file though.

—
Reply to this email directly or view it on
GitHubhttps://github.com/kennethreitz/requests/issues/1468#issuecomment-21244742
.


Aha, awesome, totally missed that. =) In that case, I pronounce us legally in the clear!
Improve documentation of netrc authentication

The documentation does not make it clear that when the credentials
from netrc are used, Requests authenticates with HTTP Basic Auth.

I just spent ages trying to figure out why it wasn't working, and
it was because although the credentials in the netrc were correct,
the server actually required HTTP Digest Auth.

Add a section in the authentication documentation to make it clear
that HTTP Basic Auth is used.


Hi @dpursehouse, thanks for this PR!

I think this would be better placed as a subheading under the `Basic Authentication` section. Do you mind moving it?


@Lukasa No problem.  I just added a new commit.


Awesome, that looks good to me! Thanks a bunch! :cake:
Fix #1322: Add note in docs about None not being sent as data

In the case:

  payload = {'key1': 'value1', 'key2': 'value2', 'key3': None}
  r = requests.get("http://httpbin.org", params=payload)

the parameter `key3` will not be sent as a parameter in the URL.

Mention this in the documentation.


Awesome, thanks for this @dpursehouse! :star2:
https POST works on Mac but fails on Windows

Trying to post an API call to voeis.msu.montana.edu, a hydrology data management site running on https. Script works fine on my MacBookPro (v 10.8.2)... but fails miserably on Windows 7 (two different machines):

\requests\adapters.py", line 331, in send raise SSLError(e)
requests.exceptions.SSLError: [Errno 1] _ssl.c:504: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed

I am pretty sure I was having the same issue when I was trying to use poster and urllib2 for the same purpose.


So the problem here is that the SSL certificate presented by `voeis.msu.montana.edu` is failing certificate validation. You can temporary work around the problem by setting `verify=False` on your requests calls.

I'm pretty sure this means their root certificate isn't in our local `cacerts.pem` file, which means that likely needs updating.


Hmm, we actually do have GeoTrust Global CA in our certs file, and their certificate is super-old, so that seems unlikely to be it.


I did use the verify=False workaround for now... but that is only a workaround. What I don't understand is why it works great on my Mac, but fails gloriously on Windows. What is different with how they handle certs? Is one trying to use your **cacert.pem**, and the other trying to use something else?


my first guess would be openssl differentes 


I can reproduce this on arch. It seems the server doesn't include the intermediary certificate `GeoTrust SSL CA`.
Maybe openssl on OS X caches certificates like browsers.
You should be able to extract the intermediary certificate with a browser in PEM-Format and add this to `cacert.pem`.


@t-8ch did you mean for us to add it to the copy of `cacert.pem` in requests or just for @heetderks to do that for himself? If the former, I'm not sure we should be adding extra certs to that file, especially if it seems to be a server issue.


I meant @heetderks. The certificate isn't included with Firefox or Chrome, so I don't think requests should ship it by default.
As you said, this is a server issue.