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

fixed #1042 -- corrected an import #1043

Merged
merged 1 commit into from
Jun 1, 2017
Merged

fixed #1042 -- corrected an import #1043

merged 1 commit into from
Jun 1, 2017

Conversation

alex
Copy link
Member

@alex alex commented Jun 1, 2017

No description provided.

@dstufft
Copy link
Member

dstufft commented Jun 1, 2017

👍

@glyph
Copy link

glyph commented Jun 1, 2017

Presumably this should be force-merged since the former released version's breakage is what's breaking the build?

@alex
Copy link
Member Author

alex commented Jun 1, 2017

That does appear to be the case.

@dmsimard dmsimard mentioned this pull request Jun 1, 2017
@dmick
Copy link

dmick commented Jun 1, 2017

Please merge and release asap

@jakepruitt
Copy link

^ Seconded, we really would appreciate this getting merged as soon as possible.

@davidism
Copy link

davidism commented Jun 1, 2017

I'm sure they're already aware that everyone would like this merged, so can we please not all chime in with the equivalent of "+1"? It just spams everyone's notifications, which I want to watch to see when actual progress is made. Use the reaction buttons.

@digitalresistor
Copy link

The reaction buttons don't get an email sent to any maintainers/watchers of the project. Adding comments does.

It really is not acceptable to break the world, then when notified leave the breakage for hours (at this point it has been 4+ hours)

@alex
Copy link
Member Author

alex commented Jun 1, 2017

You're almost certainly sending notifications to someone who is asleep, which isn't productive.

@fake-name
Copy link

fake-name commented Jun 1, 2017

I kinda feel that for packages as core to basically everything python as pip is, maybe schedule the release for the start of the day, rather then right before leaving to go to sleep?

Having someone who can do an emergency rollback/fix available for a few hours after a new version is released doesn't seem like a huge overhead. Just put that late-afternoon release off to the next morning.


As an aside, I do find it amusing that all the failing CI builds I looked at are failing because they can't install packages due to this bug. So much for testing.

@kornicameister
Copy link

kornicameister commented Jun 1, 2017

Seems like this bug affects majority of OpenStack bug.

Guess this is how it sounds like by looking at the IRC discussions and my local experiences from today's morning.

@mosquito
Copy link

mosquito commented Jun 1, 2017

Guys it's super critical for me. All deployment process was broken.

@mosquito
Copy link

mosquito commented Jun 1, 2017

@jaraco all python ecosystem was broken. Please merge it ASAP!!!

@punkr0ck
Copy link

punkr0ck commented Jun 1, 2017

When installing Let's Encrypt, I get this error (not sure if it's related to this topic):

Installing Python packages...
  Failed building wheel for pip
  Failed building wheel for wheel
Command "/root/.local/share/letsencrypt/bin/python2.7 -u -c "import setuptools, tokenize;__file__='/tmp/pip-qv6is_-build/setup.py';f=getattr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" install --record /tmp/pip-S16j4X-record/install-record.txt --single-version-externally-managed --compile --install-headers /root/.local/share/letsencrypt/include/site/python2.7/pip" failed with error code 1 in /tmp/pip-qv6is_-build/
Traceback (most recent call last):
  File "/tmp/tmp.rKuEI000oW/pipstrap.py", line 146, in <module>
    exit(main())
  File "/tmp/tmp.rKuEI000oW/pipstrap.py", line 133, in main
    shell=True)
  File "/usr/lib/python2.7/subprocess.py", line 574, in check_output
    raise CalledProcessError(retcode, cmd, output=output)
subprocess.CalledProcessError: Command 'pip install --no-index --no-deps -U /tmp/pipstrap-Ln63ah/pip-8.0.3.tar.gz /tmp/pipstrap-Ln63ah/setuptools-20.2.2.tar.gz /tmp/pipstrap-Ln63ah/wheel-0.29.0.tar.gz' returned non-zero exit status 1

@@ -4,7 +4,7 @@

import platform

import six
from setuptools.extern import six

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why using six???

In package six the Python version is determined by calling sys.

PY2 = sys.version_info[0] == 2
See: https://bitbucket.org/gutworth/six/src/92e1c746e1abfbdb94705b821bc93766083efc54/six.py?at=default&fileviewer=file-view-default

Best solution is to use "sys" to check if it is python 2 version.

If it must be version 2.7 check for major and minor version!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert 1d928cb ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually reverting should looks better.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, reverting is a much cleaner solution.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all people still wait, if you can please revert this shortly

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

six is already used elsewhere, so it makes sense to use it here as well (after the import is corrected).

@westsouthnight This was worked around by pulling the affected version from PyPI, so things should be fine again for now.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have seen that it was used in other parts to check if an object is of the type "isinstance(value, six.string_types)". Does that mean that this part of the code must have six? I just can't see that "import sys" must be replaced by six. It doesn't solve a problem in my opinion.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fwdevmobile It doesn't mean it must have six. But it makes sense for it to use it when available, to - even if only for a tiny bit - increase readability and reduce technical debt. It's unfortunate that there was a mistake in it, but generally, it - IMHO - is a slight improvement.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree with the new import will solve all deployment issues. Please merge.

from setuptools.extern import six

nicolaiarocci added a commit to pyeve/events that referenced this pull request Jun 1, 2017
This reverts commit 461e77e.

Looks like setuptools has a problem which breaks CI builds,
see: pypa/setuptools#1043
@jaraco jaraco merged commit fa38086 into pypa:master Jun 1, 2017
@anthrotype
Copy link

Thanks @jaraco!

@alex alex deleted the patch-1 branch June 1, 2017 12:56
@jaraco
Copy link
Member

jaraco commented Jun 1, 2017

Thanks for providing this change. I apologize that I wasn't available to review and cut a release. I have made the release process thoroughly documented and readily available to anybody on PyPA. I typically also leave time after cutting a release to respond to any regressions, but due to the two botched attempts, I ran out of time and was relying on the passing tests to assure the release. Perhaps setuptools test suite needs a test that it can be installed in a clean environment.

@thanatos
Copy link

thanatos commented Jun 1, 2017

I apologize

I for one don't believe I'm owed an apology here. We've all broken the build/production from time-to-time, but I don't think I'm capable of imaging the enormous pressure that must come with breaking everyone's builds. 😉 Python, and the tooling around it like PyPI, setuptools, and pip, are all free — but the value we as developers derive from it is enormous. Thanks for taking the time to be a maintainer of such a critical piece of infrastructure.

A patch for the problem was available within minutes, and I got an instant response on IRC channel. I think if anything, we owe you an apology for the incessant clamoring for an instant fix/release.

I have made the release process thoroughly documented and readily available to anybody on PyPA.

👍 Thanks for this. There were also several good suggestions for us as users on the issue.

@hynek
Copy link

hynek commented Jun 2, 2017

@jaraco unless you have a paid SLA with anyone you don’t owe us any apologies. It’s incredible how robust Python’s packaging has become due to PyPA’s tireless efforts. So thank you once more! The complaining wouldn’t be half as loud if the baseline reliability weren’t so good.


Those complaining loudly about broken deployments should be thankful for this reminder that everything can break and it’s part of your jobs to deal with it. Google goes internally as far as introducing outages if a system has been too reliable for too long (see “Don’t overachieve” in the SLO chapter of the SRE book).

This one was a minor incident that got fixed within a short period. There are totally scenarios where infrastructure you take for granted – and have no influence over! – goes away for a week or longer. How will you cope?

@willingc
Copy link

willingc commented Jun 2, 2017

@jaraco Thank you! I agree with @hynek that you and PyPA's maintainers and contributors have done a fantastic job the past few years. Thank you for being professional, responsive, helpful, and enhancing the user experience tremendously. 🌷 🌹 🌷

@hynek Thank you for adding thoughtful perspective on this too.

@mosquito
Copy link

mosquito commented Jun 2, 2017

I apologize if I offended someone. However, I wrote only the facts. The ecosystem was actually broken. The worst thing is that there was no way to get around this problem for me. @jaraco did everything right after all. Everyone could make such a mistake. Setuptools is a very reliable package. Thank you.

@digitalresistor
Copy link

@jaraco and other setuptools maintainers I sincerely apologise for my comment, and the strife it has caused.


I've been getting a lot of flack for my comment above, especially on the Twitter sphere, and in private via email. Lot of people saying people should have known to pin the package or anything along those lines.

Let me state up front that when I posted that comment I was incredibly frustrated, mainly because it wasn't until this started happening that I found out that virtualenv by default -- upon creation of a new virtualenv -- will happily go fetch and download the latest version from pypi and install it. Everything else in production at $WORK is pinned to a private PyPI server that contains the packages we want with the versions we want, however unlike python3 -mvenv there was no realisation (from anyone on the team) that virtualenv would go download stuff from the internet.

Ansible with a pip statement will run virtualenv if you provide a virtualenv path, there is no flag to disable the default behaviour (to for example add --no-download), tox uses virtualenv internally, there is no way to pass --no-download. There is no way to specify with the virtualenv command to go fetch from an alternative PyPI location (-i flag is missing, I've opened an issue for this) other than exporting an environment flag that is not well documented (and happens to be consumed by pip during a run of virtualenv).

The virtualenv usage docs: https://virtualenv.pypa.io/en/stable/userguide/#usage don't specify it is going to download code from the internet. I was fully expecting it to deploy with the setuptools it bundles.

Sure there are work-arounds, and I am sure there is a place in the virtualenv docs where it describes that by default it will go fetch things from PyPI, but the amount of long-time Python developers that were not aware of the default virtualenv behaviour within not just my team but people that I've spoken to in the community makes this a much bigger problem.


I maintain a variety of open source projects, I know how thankless of a job it is, and had I put on my maintainer hat when I posted the comment above I wouldn't have hit the comment button, I don't envy the position that setuptools has by being such an important part of the Python eco-system and thus being in a place where breakage is felt so far and wide.

@mmerickel
Copy link

Expanding on @bertjwregeer's excellent point about --no-download, the issue here is that even if we pinned setuptools in our requirements.txt we would still be broken because the virtualenv package installed the latest setuptools already which broke the virtualenv. We would not be able to downgrade to our pinned version after that. Secondly, pip freeze by default excludes setuptools making it that much harder and non-obvious to pin it - even when a package in your tree depends on setuptools explicitly for runtime pkg_resources features. In this new age of setuptools bumping the major version number frequently it's possible these defaults should be re-evaluated to make things easier to pin and to create more stable environments without requiring things like VIRTUALENV_NO_DOWNLOAD=1 and pip freeze --all.

@dstufft
Copy link
Member

dstufft commented Jun 2, 2017

We're likely going to be able to stop excluding setuptools in pip freeze soon (or installing it by default inside of a virtual environment at all). The work done in PEP 518 opens up a path to being able to do that. We'll likely want some time with the feature actually being released before we get to that point, but that's what I expect to happen in the near future.

@shadowmint
Copy link

This isn't really the place for this discussion, but if you recall #940 from a couple of months ago, this isn't the first time virtualenv behaviour has been broken with setuptools in recent history.

Some kind of mitigation for this repeated type of failure should be put in place.

Where's the best place to take this discussion to move it forward?

@leorochael
Copy link
Contributor

As someone who, using zc.buildout, has been exposed to breakages arriving from eventual mismatch between setuptools and buildout, here is what I use for repeatable setups:

vitualenv --no-setuptools --no-pip sandbox
sandbox/bin/python <(curl https://bootstrap.pypa.io/get-pip.py) --upgrade setuptools==33.1.1 pip==9.0.1

The trick is that get-pip.py, when passed additional arguments, behaves like pip install, so you can now make it install exactly the packages you need, in whatever version you need from whatever location you need, and virtualenv won't have gone over the internet to fetch some arbitrary version of setuptools or pip.

tanaypf9 pushed a commit to tanaypf9/pf9-requirements that referenced this pull request May 20, 2024
Patch Set 2:

The fix for setuptools was merged: pypa/setuptools#1043

Patch-set: 2
Reviewer: Gerrit User 20033 <20033@4a232e18-c5a9-48ee-94c0-e04e7cca6543>
Label: Verified=0
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 this pull request may close these issues.