Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade urllib3 dependency #75

Closed
acordiner opened this issue Jun 13, 2017 · 16 comments
Closed

Upgrade urllib3 dependency #75

acordiner opened this issue Jun 13, 2017 · 16 comments

Comments

@acordiner
Copy link
Contributor

Could the urllib3 dependency be upgraded? This package depends on urllib3 <= version 1.16. However, 1.16 is now 12 months old and the latest version is 1.21.1. Depending on an older version of urllib3 makes it difficult to use django-revproxy in a project that uses other packages that depend on urllib3; for example, the latest version of python-requests depends on urllib3 version 1.21: https://github.com/requests/requests/blob/master/setup.py#L47

@seocam
Copy link
Contributor

seocam commented Jun 13, 2017

Can you try to update the version and run the tests to make sure it still working?

@seocam
Copy link
Contributor

seocam commented Jun 27, 2017

Looks like it's not working properly with newer versions because of their implementation of HTTPHeaderDict (https://github.com/shazow/urllib3/blob/118a936e1a44598aa2b2eed0e361310e05e47c6c/urllib3/_collections.py#L101). Last time this happened I had to work on there to fix what we needed. That might be the case again.

@brianmay
Copy link
Collaborator

brianmay commented Jul 6, 2017

There are problems with the latest urllib3 and the custom pool manager not taking a request_context parameter:

Traceback (most recent call last):
  File "/Users/brianmay/.virtualenvs/myro/myrocc/lib/python3.6/site-packages/django/core/handlers/exception.py", line 41, in inner
    response = get_response(request)
  File "/Users/brianmay/.virtualenvs/myro/myrocc/lib/python3.6/site-packages/django/core/handlers/base.py", line 187, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/Users/brianmay/.virtualenvs/myro/myrocc/lib/python3.6/site-packages/channels/handler.py", line 243, in process_exception_by_middleware
    return super(AsgiHandler, self).process_exception_by_middleware(exception, request)
  File "/Users/brianmay/.virtualenvs/myro/myrocc/lib/python3.6/site-packages/django/core/handlers/base.py", line 185, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/local/Cellar/python3/3.6.1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/contextlib.py", line 53, in inner
    return func(*args, **kwds)
  File "/Users/brianmay/.virtualenvs/myro/myrocc/lib/python3.6/site-packages/django/views/generic/base.py", line 68, in view
    return self.dispatch(request, *args, **kwargs)
  File "/Users/brianmay/.virtualenvs/myro/myrocc/lib/python3.6/site-packages/braces/views/_access.py", line 201, in dispatch
    request, *args, **kwargs)
  File "/Users/brianmay/.virtualenvs/myro/myrocc/lib/python3.6/site-packages/revproxy/views.py", line 200, in dispatch
    proxy_response = self._created_proxy_response(request, path)
  File "/Users/brianmay/.virtualenvs/myro/myrocc/lib/python3.6/site-packages/revproxy/views.py", line 157, in _created_proxy_response
    preload_content=False)
  File "/Users/brianmay/.virtualenvs/myro/myrocc/lib/python3.6/site-packages/urllib3/poolmanager.py", line 311, in urlopen
    conn = self.connection_from_host(u.host, port=u.port, scheme=u.scheme)
  File "/Users/brianmay/.virtualenvs/myro/myrocc/lib/python3.6/site-packages/urllib3/poolmanager.py", line 227, in connection_from_host
    return self.connection_from_context(request_context)
  File "/Users/brianmay/.virtualenvs/myro/myrocc/lib/python3.6/site-packages/urllib3/poolmanager.py", line 240, in connection_from_context
    return self.connection_from_pool_key(pool_key, request_context=request_context)
  File "/Users/brianmay/.virtualenvs/myro/myrocc/lib/python3.6/site-packages/urllib3/poolmanager.py", line 261, in connection_from_pool_key
    pool = self._new_pool(scheme, host, port, request_context=request_context)
TypeError: _new_pool() got an unexpected keyword argument 'request_context'

For a similar bug report in requests, see psf/requests#4010

@brianmay
Copy link
Collaborator

brianmay commented Jul 6, 2017

Also, note that upgrading requests will result in urllib3 being upgraded too. Even if it does break this dependency.

aausch added a commit to aausch/django-revproxy that referenced this issue Jul 24, 2017
I believe this fixes jazzband#75
aausch added a commit to aausch/django-revproxy that referenced this issue Jul 24, 2017
@albertored
Copy link

I tried the fix from @aausch and it seems to work (I'm using urllib3 1.22).
It has not been accepted yet because is still in testing?

@regit
Copy link

regit commented Sep 5, 2017

Any news on this ? there is more and more problems with keeping an old urllib3

@eiva
Copy link

eiva commented Mar 7, 2018

It's become a problem...

@studio-salamander
Copy link

Hi for all!
Author, when will be fixed this issue:
django-revproxy 0.9.14 has requirement urllib3<1.17,>=1.12, but you'll have urllib3 1.22 which is incompatible.

@Blisk
Copy link

Blisk commented May 8, 2018

I still have the same problem on centos 7
django-revproxy 0.9.14 has requirement urllib3<1.17,>=1.12, but you'll have urllib3 1.21.1 which is incompatible.

@brianmay
Copy link
Collaborator

Version 0.9.14 included the following change:

class PoolManager(PoolManager_):
-    def _new_pool(self, scheme, host, port):
+    def _new_pool(self, scheme, host, port, request_context=None):
         """
         Create a new :class:`ConnectionPool` based on host, port and scheme.
         This method is used to actually create the connection pools handed out

Unfortunately, it did not update the bad requirements in setup.py.

Is this change still required?

Is django-revproxy now compatible with urllib 1.22? If so, How can we fix the requirement in setup.py?

If django-revproxy is still incompatible with urllib 1.22, how do we get this change merged?

Is this project dead?

@Herst
Copy link

Herst commented May 23, 2018

Is this project dead?

Ping @seocam, @rougeth, and @matheus-morfi.

@seocam
Copy link
Contributor

seocam commented May 23, 2018

Hello @Herst and @brianmay!

Honestly I'm not having much time to maintain it. If you or anyone here is interested in help let me know and I'll add you as maintainers as well.

@brianmay
Copy link
Collaborator

brianmay commented May 23, 2018 via email

brianmay pushed a commit to brianmay/django-revproxy that referenced this issue May 24, 2018
@brianmay
Copy link
Collaborator

This version fixes all known issues, and all tests pass: master...brianmay:master

If/when I get push access to git, I also plan on merging #76.

@seocam
Copy link
Contributor

seocam commented May 24, 2018

@brianmay for the sake of clarity would you mind doing a PR for each known issue instead of merging your master here?
Also please remember to update the Changelog! ;)

Your access is granted.

@brianmay
Copy link
Collaborator

brianmay commented May 24, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants