-
Notifications
You must be signed in to change notification settings - Fork 388
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
Remove pypy constraint for urllib3 #826
Conversation
@hartwork - This might not be the right solution so you might not want to accept this PR... but please provide opinion if you can. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #826 +/- ##
=======================================
Coverage 92.32% 92.32%
=======================================
Files 27 27
Lines 1811 1811
Branches 338 338
=======================================
Hits 1672 1672
Misses 92 92
Partials 47 47 ☔ View full report in Codecov by Sentry. |
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.
Hi @benwah,
this will need dropping of exclude…
vcrpy/.github/workflows/main.yml
Lines 38 to 39 in acc1014
- python-version: "pypy-3.10" | |
urllib3-requirement: "urllib3>=2" |
…to prove that it works (if it does). Thanks!
@benwah PS: The Poetry aspect is interesting, it might indeed be a bug there, I plan to have a closer look later. |
Let's see how this goes.. I removed the |
@hartwork Yeah let's close the PR. In your opinion is this a Poetry bug? |
@benwah good, will do.
I reproduced it locally now with CPython 3.10 where VCR.py's…
…should by no means block an update to urllib3 >=2 but it does: # poetry add urllib3
Using version ^2.2.1 for urllib3
Updating dependencies
Resolving dependencies... (0.1s)
Because vcrpy (6.0.1) depends on urllib3 (<2)
and no versions of vcrpy match >6.0.1,<7.0.0, vcrpy (>=6.0.1,<7.0.0) requires urllib3 (<2).
So, because demo123 depends on both vcrpy (^6.0.1) and urllib3 (^2.2.1), version solving failed. Support for PEP 508 markers is broken in Poetry 1.8.2, I confirm. It's a pity they closed python-poetry/poetry#8996 . Seems like a misunderstanding. |
I'm not certain whether this is a bug in Poetry or with the marker in setup.py.
Context:
platform_python_implementation
marker incorrectly python-poetry/poetry#8996A
platform_python_implementation
is specified asPyPy
for aurllib3 <2
constraint: https://github.com/kevin1024/vcrpy/blob/master/setup.py#L57But in essence, if you depend on
urllib3 >= 2
and use CPython + Poetry, you can't install vcrpy.See sample poetry-compatible
pyproject.toml
When trying to install this with
poetry
:So I'm thinking either the
platform_python_implementation =='PyPy'"
marker is incorrect or Poetry isn't handling it as it should.