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

Inconsistent urllib3 version requirements between setup.py and __init__.py #4673

Closed
GPMueller opened this issue Jun 6, 2018 · 24 comments
Closed

Comments

@GPMueller
Copy link

With ac944b7 the maximum allowed version of urllib3 in setup.py was increased to 1.23.
However, in https://github.com/requests/requests/blob/master/requests/__init__.py#L57-L63 the version is still required to be <= 1.22.

This is a bug since the release of requests 1.23 yesterday: https://pypi.org/project/urllib3/#history

Expected Result

Installation of requests via pip should work with urllib3 version 1.23

Actual Result

e.g.

/usr/local/lib/python2.7/dist-packages/requests/__init__.py:80: RequestsDependencyWarning: urllib3 (1.23) or chardet (3.0.4) doesn't match a supported version!
  RequestsDependencyWarning)
ContextualVersionConflict: urllib3 1.23

Reproduction Steps

pip install urllib3==1.23
pip install requests
import requests
@yan12125
Copy link
Contributor

yan12125 commented Jun 6, 2018

Maybe #4669 should be reverted until urllib3 is fully supported.

@GPMueller
Copy link
Author

It looks to me like it should suffice to simply change the __init__.py to allow v1.23 of urllib3, but I'm not sure since I have not tested it.

@rblaine95
Copy link

rblaine95 commented Jun 6, 2018

Just tested changing __init__.py:63: assert minor <= 22 to <= 23 and then tested with aurman -Syu to see if the error came back.

There's no error anymore, however I'm not sure if just simply incrementing that value could cause errors further down the road.

Edit: I'll update this if I encounter any errors. So far, I haven't come across anything.

@nateprewitt
Copy link
Member

Hey everyone, to be clear this isn't a bug but rather a specific design decision. We cannot guarantee support for new releases of urllib3 before they've been tested with the Requests code base. In the event of a new release we haven't tested you will see a warning message informing you about the issue.

We are currently slated to release Requests 2.19 in the next week or two which will move the warning pin. Our standard procedure is to wait a week or so and see if any major bugs are raised in urllib3's new release. Things will continue to work fine with 1.22 in the meantime.

@valignatev
Copy link

My program relied on stdout output, and requests warning broke it. Good to know that issue is known and will be fixed soon :)

@binarykitchen
Copy link

Happens on my server too and worse, it's breaking my crontab to automatically generate certificates (certbot). The sooner we can fix and publish the better ...

@jessemyers
Copy link

jessemyers commented Jun 7, 2018

I totally understand the motivation of this design decision to generate warnings on untested urllib3 versions... but this behavior is definitely breaking in some cases (and not just a warning).

Simple test case; create a distribution with an entry_point. For example:

#!/usr/bin/env python
from setuptools import find_packages, setup

setup(
    name="test",
    version="0.1.0",
    packages=find_packages(),
    install_requires=[
        "requests==2.18.4",
        "urllib3==1.23",
    ],
    entry_points={
        "example": [
            # it doesn't matter if test.foo exists
            "foo = test.foo",
        ],
    },
)

Create a test program that resolves this entry point (we don't even have to load it):

#!/usr/bin/env python
from pkg_resources import iter_entry_points


for entry_point in iter_entry_points(group="example"):
    entry_point.require()

Then install the distribution:

python3 -m venv test
source test/bin/activate
pip install -U -e

This works fine. Even though we have version conflicts, pip lets us continue.

But then try to ask setuptools to resolve the entry point:

> python3 ./test.py
Traceback (most recent call last):
  File "./test.py", line 6, in <module>
    entry_point.require()
  File "/private/tmp/test/test/lib/python3.6/site-packages/pkg_resources/__init__.py", line 2346, in require
    items = working_set.resolve(reqs, env, installer, extras=self.extras)
  File "/private/tmp/test/test/lib/python3.6/site-packages/pkg_resources/__init__.py", line 783, in resolve
    raise VersionConflict(dist, req).with_context(dependent_req)
pkg_resources.ContextualVersionConflict: (urllib3 1.23 (/private/tmp/test/test/lib/python3.6/site-packages), Requirement.parse('urllib3<1.23,>=1.21.1'), {'requests'})

TL/DR; putting an upper bound on urllib3 versions is breaking for any application that uses entry points in this way and "happens" to upgrade dependencies. That might be your intention, but this behavior definitely goes beyond "warnings" and definitely breaks some workflows (e.g. ours).

@nateprewitt
Copy link
Member

Hi @jessemyers, thanks for taking the time to present your case. While we definitely want to be sensitive to breaking anyone's code, I'm not sure this constitutes as "breaking". At least not any more so than choosing to pass an invalid parameter to a function is breaking.

We haven't changed anything about the requirements with Requests 2.18.4. They state Requests requires a urllib3 version between 1.21.1 and 1.22.x to function as expected. Having an upper bound pin isn't an uncommon occurrence in libraries for compatibility purposes.

The example provided is a case of a user choosing to change input parameters (by specifying a hard requirement for a non-compatible version) and pkg_resources correctly failing because the given requirements aren't compatible. When you call require on an EntryPoint, you're invoking a specific contract to resolve the extras associated with it, and in turn, resolve the top level requirements as well. No code in Requests is breaking at this point, so I'm not sure we can do much about that for a general case. The same thing would happen if you chose to install urllib3==1.20 or any other two conflicting packages with any library.

The reason pip doesn't fail in this case is because it doesn't have an internal resolver, which is being tracked in pypa/pip#988.

@jessemyers
Copy link

Thanks for the prompt response @nateprewitt

My primary point was that pinning an upper bound on a dependency version goes beyond "a warning message" as was described above in your explanation of policy.

To be more specific, we use the following workflow:

  • Release all of our software on a regular cadence (currently: weekly)
  • As part of the release process, create release branches in which dependencies are frozen to the last known good version, unless explicitly changed
  • As part of our regular (non-release) process, resolve all dependencies to the most recent version compatible with pip unless explicitly forbidden

This workflow works very well in practice as it means we have stable, reproducible releases and stay on top of the latest changes to all of our dependencies. It obviously does not prevent several of our dependencies introducing conflicting transitive dependencies. We are quite used to addressing this sort of error across many tens of applications at once, but that doesn't mean that it is a pleasant experience. (And we see this sort of error most frequently with requests)

From our point of view, any library that pins a version upper bound for a common library can be breaking. We don't seriously expect every (any?) open source project to adapt to our needs here, though we would certainly encourage anyone who will listen to rely on functional tests instead of version upper bounds to prevent regressions.

TL/DR I completely understand that our preference is not your policy. I hope you understand why we see pinned version upper bounds as something more serious than a "warning".

@nateprewitt
Copy link
Member

@jessemyers, yeah, this is the first time in almost a year, and the third time ever we've had a release with this process. We previously owned both the release of urllib3 and Requests so it was a bit easier to sync these things. We'll take your suggestions into account if we review the policy with recent changes. Thanks again for clarifying your issues, it's appreciated feedback!

@teuneboon
Copy link

teuneboon commented Jun 7, 2018

Good to see this is getting fixed soon, but I'm wondering about one thing. If the __init__.py defines that version 1.21.1 or 1.22.x is required, why does setup.py basically say 1.23.x is ok as well? Shouldn't that say <1.23 then instead of <1.24?

Some of our code relied on stdout, we caught this in our test environment now, but it took a while to debug because the DependencyWarning didn't mention which version it depends on, so naturally I checked setup.py to confirm we had the right version, which we seemed to have. Maybe include the version the check_compatibility function checks for in the DependencyWarning?

@nateprewitt
Copy link
Member

@teuneboon, we have all of our individual releases tagged (2.18.4) so you can see the state of the code at that point, or look at it locally in site-packages where it's installed.

We can definitely look at making the exception message clearer, since there may not be an immediately obvious way to determine how to fix the conflict.

@sigmavirus24
Copy link
Contributor

To be clear, @jessemyers our position is the following:

  • Requests is critical infrastructure for many projects and the larger Python community (pip, pipenv, twine, and so many other core workflows rely on it).

  • urllib3 has the right to make minorly breaking changes to itself in between releases. Requests digs into urllib3 in certain places for certain workflows. Given that, we have to ensure things don't break when a new version of urllib3 came out, and we no longer vendor urllib3 into Requests, we need to be certain that the rest of the world won't burst into flames.

No software can guarantee future proof compatibility. We're very explicitly stating what we support. We could leave the upper bound off but we'd be implicitly supporting everything urllib3 releases. And when that would break, it would inevitably end up with us being painted as the worst people in the world.

Furthermore, there are two of us working on this project actively and both of us have very little time to keep up with other projects' release schedules. The most honest way we can communicate what we are capable of supporting at a given time is with these version constraints. We're far from the only open source project to take this stance. It would seem, then, that the more projects your project(s) use, the more they will encounter this and the more you will need to take a more measured approach to upgrading your dependencies.

That would be easier if there was a real dependency resolver in pip, but I don't think we should compromise the stability of the project because there isn't a real dependency resolver in pip.

@teuneboon
Copy link

@nateprewitt it seems like the timing we upgraded our packages in was unfortunate, and the issues we had didn't have anything to do with requests in the end. When I saw that our code was giving warnings about dependencies of requests and I went to the Github issues page and I saw this issue I (wrongly) assumed this was our issue as well(and I checked the master branch instead of the tagged release which made the confusion greater).

In the end we only had an issue with pip dependency resolving and the order in which we installed packages which gave us urllib 1.23 for requests 2.18.4.

Making the warning clearer would be great though, thanks!

@glyph
Copy link

glyph commented Jun 7, 2018

Another issue with this upper-version-bound pin is that it causes services like @requires to generate pull requests that fail tests, since they upgrade urllib3 in downstream requires.txt files even though requests may not be ready for it. Arguably this is a bug in @requires and it should have its own sat solver to generate the most recent possible viable set of versions, but practically speaking it can block security updates on other projects trying to use modern tooling.

@sigmavirus24
Copy link
Contributor

practically speaking it can block security updates on other projects trying to use modern tooling.

Can you explain a bit more about this? Do you mean that the update for urllib3 is being included in a batch of other updates in the same PR and that's blocking those somehow?

dsoper2 pushed a commit to dsoper2/intersight-python that referenced this issue Jun 8, 2018
openstack-gerrit pushed a commit to openstack/requirements that referenced this issue Jun 10, 2018
subunit lower-constraint raised because stestr now has a minimum of 1.3.0
networkx kept at 1.11 for breakage
  * https://bugs.launchpad.net/openstack-requirements/+bug/1718576
eventlet kept at 0.20.0 for breakage
google-auth and google-api-python-client kept at current for cinder breakage
  * https://bugs.launchpad.net/openstack-requirements/+bug/1775218
Sphinx kep at 1.6.5 for docs breakage
pyldap, removed post1 again
urllib3 is being held back because of requests
  * psf/requests#4673
pexpect is being held back for ansible-runner
  * ansible/ansible-runner#44

Change-Id: I7e4a927db7a20eaa7ee2921868df01c2acf5aa2e
@nateprewitt
Copy link
Member

Alright, 2.19.0 is out the door! Closing this out since it should be resolved now.

openstack-gerrit pushed a commit to openstack/requirements that referenced this issue Jun 13, 2018
networkx kept at 1.11 for breakage
  * bugs.launchpad.net/openstack-requirements/+bug/1718576
google-auth and google-api-python-client kept at current for cinder breakage
  * bugs.launchpad.net/openstack-requirements/+bug/1775218
eventlet kept at 0.20.0 for breakage
Sphinx kep at 1.6.5 for docs breakage
pyldap, removed post1 again
urllib3 is being held back because of requests
  * psf/requests#4673
idna is being held back by requests as well
  * psf/requests#4681
pexpect is being held back for ansible-runner
  * ansible/ansible-runner#44

Change-Id: I90626ff9d65ab0df875d2c5a2d058a472631f1b6
mattmb added a commit to Yelp/paasta that referenced this issue Jun 13, 2018
openstack-gerrit pushed a commit to openstack/project-config that referenced this issue Jun 13, 2018
I was looking at wheel build failures, and in some cases builds fail
because they're not constrained to versions already in the
constraints.

For example; pylxd failed to build a wheel [1] due to [2], but
upper-constraints.txt would have kept urllib to 1.22.  Similarly
several django plugins fail on python2 as they try to bring in Django2
which only works with python3.

Pass the upper-constraints.txt to the wheel build to ensure consistent
building.

[1] http://logs.openstack.org/periodic/git.openstack.org/openstack/requirements/master/publish-wheel-mirror-centos-7/723e75b/python2/failed.txt

[2] psf/requests#4673

Change-Id: I788994b69afd2769489454a3b16f84bea4c56e59
@stevelle
Copy link

Thank you again to @nateprewitt and @sigmavirus24 for the careful balancing act here, both in navigating the updates to urllib3 without vendoring and for your community support here.

@PatrickSteil
Copy link

Sometimes it's because of (for example) Kaspersky... Turn it off, and pip install the package

@kirillgroshkov
Copy link

Seems like this issue broke installation of awscli

@nateprewitt
Copy link
Member

Hi @kirillgroshkov, this has been fixed for a few months now. Please make sure you’ve updated Requests to the latest version.

@iamakulov
Copy link

Seems like this issue broke installation of awscli

Also has been experiencing this with awsebcli on my CircleCI instances since 2-3 weeks ago. Upgrading pip and requests before installing awsebcli solved the issue.

Here’s what my CI script was doing before:

sudo apt-get update && sudo apt-get install -y python-pip python-dev build-essential
pip install awsebcli --user

Here’s what it’s doing now:

sudo apt-get update && sudo apt-get install -y python-pip python-dev build-essential
export PATH=~/.local/bin:$PATH
sudo pip install --upgrade pip
sudo pip install --upgrade requests
pip install awsebcli --upgrade --user

@kirillgroshkov
Copy link

Here's how I solved it yesterday:

pip install --upgrade "urllib3==1.22" awscli awsebcli

@virtualbeck
Copy link

virtualbeck commented Nov 7, 2018

pip install --upgrade "urllib3==1.22" awscli awsebcli

Worked for me, had to add --userto the end to make it work

@psf psf locked as resolved and limited conversation to collaborators Nov 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

16 participants