-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix import error introduced in 21.1 #9835
Conversation
.github/workflows/ci.yml
Outdated
@@ -93,6 +93,7 @@ jobs: | |||
matrix: | |||
os: [Ubuntu, MacOS] | |||
python: | |||
- 3.6.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea here of pinning to 3.6.0 to make sure we don't regress, but doing so will (if I'm reading the docs correctly) result in 3.6.0 getting downloaded, rather than using the preinstalled version of 3.6. That will slow down our CI a bit, and we have an ongoing fight to speed up our CI. @pypa/pip-committers awhat do you think? Is this worthwhile?
Also, we really don't need both 3.6.0 and 3.6...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More than happy to remove this if you're comfortable. I added it as a way to demonstrate that this patch resolves the issue, but perhaps the value is outweighed by the interest in speeding up CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea in principle, but let's see what the other committers think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could have scheduled pipelines where you enable such configuration(s).. ?
Also, it looks like black wants to mess with the changed code 🙁 |
Not black, isort. From our documentation:
We explicitly state that we support the latest patch version releases of CPython versions and other versions are on a best-effort basis. I'm definitely a strong -1 on adding 3.6.0 to our CI. I'm a bit bleh on this PR overall TBH. I don't think this fix is worth making unless it's a quick bugfix release and we're yanking the 21.1 release because of this issue. And... Well, I don't think this is worth doing that much churn anyway? |
Anyway, that doesn't work either:
|
Thanks for the feedback on that: I have removed it.
Based on the principle that previous patch versions are supported on a "best effort" basis, it seems that resolving this regression would be reasonable. If the maintainers felt strongly about not wanting to make this patch, I'd advocate for updating Thoughts? |
I'm personally not going to be doing anything for < 3.6.2 support. Honestly, I couldn't care less about what we do here. 3.6.2 is from 2017 (https://www.python.org/dev/peps/pep-0494/#id1), we explicitly state that the supported Python versions are the latest patch version. Any support for significantly older versions is kindness for folks on those older versions, and I have other things to do that I care about more. I only said something here because there were changes to the CI for guaranteeing something we explicitly state the opposite of in our documentation. That change wouldn't have worked anyway, so I'm gonna shut up now. :) |
That said, I'm not the only maintainer here, so if some other maintainer does have any interest in fixing this, I'm not going to be blocking them. :) |
@pfmoore (or other maintainers): Is there any interest in reviewing and accepting this PR such that pip is compatible with all patch versions of 3.6? |
I'm personally not opposed to merging this. I'd add a comment explaining why NoReturn is under TYPE_CHECKING, so it is not accidentally removed. Can you also squash the commits? However, I'm not sure this alone warrants a bugfix release. |
+1 to merging this. We probably have a need for a patch release anyway (for the headers mismatch thing) and this fixes a real issue (albeit edge case) with virtually zero downsides. |
cb4969f
to
6d63690
Compare
Done--thank you for taking a look! |
I see no harm in merging it, so I've approved. But I'll leave the merge to someone else. By the way, this thing were I have to approve every CI run for a new contributor, not just the first one, is a complete pain in the a***. (But I'm not blaming you, @jamescurtin, just to be clear!) |
Thank you @jamescurtin |
Thank you for fixing this! May I ask when is it expected to be released? As I understand, this is going out in |
+1 for a speedy release (and thank you for the fix)! |
…oo large. Work around pypa/pip#9835. PiperOrigin-RevId: 370472029
…oo large. Work around pypa/pip#9835. PiperOrigin-RevId: 370560208
I suggest subscribing to #9761 for release-related updates. Please be mindful about making comments there though, since lots of folks have subscribed to that issue for release-related updates. |
Issue is get_pip.py is moved to pip 21.1 (https://github.com/pypa/get-pip/commits/main) which is not compatible with 3.6. Issue of pip itself is fixed as part of 21.1.1 in pip community (pypa/pip#9835). However get-pip.py is still not updated to latest pip. Also get.pip.py does not support python 3.6 version explicitly (pypa/get-pip#88) Step 15/29 : RUN curl https://bootstrap.pypa.io/get-pip.py | python3.6 ---> Running in bece31f49267 % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 100 1891k 100 1891k 0 0 9564k 0 --:--:-- --:--:-- --:--:-- 9600k Traceback (most recent call last): File "<stdin>", line 24298, in <module> File "<stdin>", line 139, in main File "<stdin>", line 115, in bootstrap File "<stdin>", line 96, in monkeypatch_for_cert File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/commands/__init__.py", line 9, in <module> File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/cli/base_command.py", line 12, in <module> File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/cli/cmdoptions.py", line 30, in <module> File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/utils/hashes.py", line 2, in <module> ImportError: cannot import name 'NoReturn' The command '/bin/sh -c curl https://bootstrap.pypa.io/get-pip.py | python3.6' returned a non-zero code: 1 How I did: Got the file from https://github.com/pypa/get-pip/tree/21.0 and added to the buildimage pin pip to the previous release 21.0.1. (Similar is done in other public repos eg: grpc/grpc-java#8115) Signed-off-by: Abhishek Dosi <[email protected]>
Issue is get_pip.py is moved to pip 21.1 (https://github.com/pypa/get-pip/commits/main) which is not compatible with 3.6. Issue of pip itself is fixed as part of 21.1.1 in pip community (pypa/pip#9835). However get-pip.py is still not updated to latest pip. Also get.pip.py does not support python 3.6 version explicitly (pypa/get-pip#88) Step 15/29 : RUN curl https://bootstrap.pypa.io/get-pip.py | python3.6 ---> Running in bece31f49267 % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 100 1891k 100 1891k 0 0 9564k 0 --:--:-- --:--:-- --:--:-- 9600k Traceback (most recent call last): File "<stdin>", line 24298, in <module> File "<stdin>", line 139, in main File "<stdin>", line 115, in bootstrap File "<stdin>", line 96, in monkeypatch_for_cert File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/commands/__init__.py", line 9, in <module> File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/cli/base_command.py", line 12, in <module> File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/cli/cmdoptions.py", line 30, in <module> File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/utils/hashes.py", line 2, in <module> ImportError: cannot import name 'NoReturn' The command '/bin/sh -c curl https://bootstrap.pypa.io/get-pip.py | python3.6' returned a non-zero code: 1 How I did: Got the file from https://github.com/pypa/get-pip/tree/21.0 and added to the buildimage pin pip to the previous release 21.0.1. (Similar is done in other public repos eg: grpc/grpc-java#8115) Signed-off-by: Abhishek Dosi <[email protected]>
* [build] Fix the snmp docker build error. (#7452) Issue is get_pip.py is moved to pip 21.1 (https://github.com/pypa/get-pip/commits/main) which is not compatible with 3.6. Issue of pip itself is fixed as part of 21.1.1 in pip community (pypa/pip#9835). However get-pip.py is still not updated to latest pip. Also get.pip.py does not support python 3.6 version explicitly (pypa/get-pip#88) Step 15/29 : RUN curl https://bootstrap.pypa.io/get-pip.py | python3.6 ---> Running in bece31f49267 % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 100 1891k 100 1891k 0 0 9564k 0 --:--:-- --:--:-- --:--:-- 9600k Traceback (most recent call last): File "<stdin>", line 24298, in <module> File "<stdin>", line 139, in main File "<stdin>", line 115, in bootstrap File "<stdin>", line 96, in monkeypatch_for_cert File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/commands/__init__.py", line 9, in <module> File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/cli/base_command.py", line 12, in <module> File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/cli/cmdoptions.py", line 30, in <module> File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/utils/hashes.py", line 2, in <module> ImportError: cannot import name 'NoReturn' The command '/bin/sh -c curl https://bootstrap.pypa.io/get-pip.py | python3.6' returned a non-zero code: 1 How I did: Got the file from https://github.com/pypa/get-pip/tree/21.0 and added to the buildimage pin pip to the previous release 21.0.1. (Similar is done in other public repos eg: grpc/grpc-java#8115) Signed-off-by: Abhishek Dosi <[email protected]>
PiperOrigin-RevId: 371166775 Change-Id: I111e81557f0129841e6bdd7c63b0e2b03be595fc
Issue is get_pip.py is moved to pip 21.1 (https://github.com/pypa/get-pip/commits/main) which is not compatible with 3.6. Issue of pip itself is fixed as part of 21.1.1 in pip community (pypa/pip#9835). However get-pip.py is still not updated to latest pip. Also get.pip.py does not support python 3.6 version explicitly (pypa/get-pip#88) Step 15/29 : RUN curl https://bootstrap.pypa.io/get-pip.py | python3.6 ---> Running in bece31f49267 % Total % Received % Xferd Average Speed Time Time Time Current Dload Upload Total Spent Left Speed 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0 100 1891k 100 1891k 0 0 9564k 0 --:--:-- --:--:-- --:--:-- 9600k Traceback (most recent call last): File "<stdin>", line 24298, in <module> File "<stdin>", line 139, in main File "<stdin>", line 115, in bootstrap File "<stdin>", line 96, in monkeypatch_for_cert File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/commands/__init__.py", line 9, in <module> File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/cli/base_command.py", line 12, in <module> File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/cli/cmdoptions.py", line 30, in <module> File "/tmp/tmp5fnxrz0a/pip.zip/pip/_internal/utils/hashes.py", line 2, in <module> ImportError: cannot import name 'NoReturn' The command '/bin/sh -c curl https://bootstrap.pypa.io/get-pip.py | python3.6' returned a non-zero code: 1 How I did: Got the file from https://github.com/pypa/get-pip/tree/21.0 and added to the buildimage pin pip to the previous release 21.0.1. (Similar is done in other public repos eg: grpc/grpc-java#8115) Signed-off-by: Abhishek Dosi <[email protected]>
Fixes #9831 by guarding the import of
NoReturn
in aif TYPE_CHECKING:
block because this type was only introduced in Python 3.6.2, butpip
is compatible with Python >=3.6.0