urllib3 connection pool rewrite breaks requests

[glopezmartin]

Hello

I've been testing testing some code that automatically deploys to Azure when I noticed a strange behaviour, whenever an msrest package was imported, my requests calls stopped working.

I'm using Python 3.4.3 on Linux, package versions are:
msrest 0.1.1
msrestazure 0.1.1

The Minimal code that produces the error:

    >>> import requests
    >>> from msrestazure.azure_active_directory import ServicePrincipalCredentials
    >>> requests.get('http://www.bing.com')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/local/lib/python3.4/dist-packages/requests/api.py", line 67, in get
        return request('get', url, params=params, **kwargs)
      File "/usr/local/lib/python3.4/dist-packages/requests/api.py", line 53, in request
        return session.request(method=method, url=url, **kwargs)
      File "/usr/local/lib/python3.4/dist-packages/requests/sessions.py", line 468, in request
        resp = self.send(prep, **send_kwargs)
      File "/usr/local/lib/python3.4/dist-packages/requests/sessions.py", line 576, in send
        r = adapter.send(request, **kwargs)
      File "/usr/local/lib/python3.4/dist-packages/requests/adapters.py", line 376, in send
        timeout=timeout
      File "/usr/local/lib/python3.4/dist-packages/msrest/pipeline.py", line 409, in urlopen
        if retries.retry_cookie and host == 'localhost':
    AttributeError: 'Retry' object has no attribute 'retry_cookie'

I've been tracing why this was happening and arrived to autorest/ClientRuntimes/Python/msrest/msrest/pipeline.py:406

def urlopen(self, method, url, body=None, headers=None,
                retries=None, *args, **kwargs):
        host = self.host.strip('.')
        if retries.retry_cookie and host == 'localhost':

You can see that retries is set by default to None and then the if checks the attribute directly, that seems odd to me, either make the retries mandatory or check with hasattr that it exists.

However this should not affect my code, but on the same file pipeline.py we find that the code is directly changing values in urllib3:

from requests.packages.urllib3.poolmanager import pool_classes_by_scheme
....
pool_classes_by_scheme['http'] = ClientHTTPConnectionPool

Accesing the pool_classes_by_scheme directly does not seems to be very object oriented and has the nasty effect of overriding ANY http connection made by requests. The code is executed at module level, so simply importing executes the override.

I'm trying to bypass this effect but using requests Sessions has not been effective. I'm planning on saving the variable pool_classes_by_scheme before and after the import and switch it as needed, but this is very ugly code.

I'm not very familiar with urllib3 but there must be a way to apply the custom pool/adapter/whatever only to azure urls and leave the others trough the default pool/adapter/whatever

[yugangw-msft]

Add @annatisch @lmazuel for comments

[annatisch]

Urgh... yes this is the ugly work around for the test workflow...
I will get this sorted asap - thanks!!

[annatisch]

#826

[devigned]

@annatisch is this fixed? Can this issue be closed?

[annatisch]

While the updated package hasn't yet been published to PyPI, the fix has been merged to master, so can probably be closed :)

[devigned]

Sounds good. I'll close it. Thank you for submitting the issue, @glamdisju. Thank you for fixing it, @annatisch.

[lmazuel]

Released
https://pypi.python.org/pypi/msrest/0.1.2