-
Notifications
You must be signed in to change notification settings - Fork 17
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 redundant greenlet dependency #51
base: main
Are you sure you want to change the base?
Conversation
I'm currently investigating kevin1024/vcrpy#848 and so running the pytest-httpbin tests on 3.13 with kevin1024/pytest-httpbin#90 but I'm blocked on greenlet supporting 3.13 |
Thanks for the PR, @graingert. I think Kevin was the one who reviewed the addition of This suggests the process cannot run in the docker image without it. I haven't taken a look at any of this yet, but we may want to verify/disprove that claim before making changes here. Unrelated, I see macOS is having a hard time, likely due to the ARM image changes from a few months ago. I'll take a look at getting that fixed so it's decoupled from this PR. |
greenlet is pulled in as a dependency of gevent, this override was only needed to get pip to install a pre release version of greenlet on 3.12. This dependency override should only have been added to the 'mainapp' extras. greenlet now supports 3.12 in a mainline release so this dependency is redundant |
I'm referring specifically to this part of the comment I linked:
Greenlet was previously deleted during that PR process and had to be readded because it broke the docker image. I agree it should come in with gevent, but there is likely something else incorrectly defined that's not resulting in all dependencies ending up in the image. |
Yeah it was added so that python 3.12 would install a pre release version of greenlet
This final release is now available |
CI currently failing due to python 3.7 missing from GitHub actions' setup-python and failing brotlicffi. Something similar to https://github.com/kevin1024/pytest-httpbin/pull/89/files#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R81 will need to be applied |
Also it looks like something is wrong with the ci matrix, this pypy3.10 build is failing due to a cpython 3.9 exception https://github.com/psf/httpbin/actions/runs/10178190245/job/28513362402?pr=51 |
Totally, I understand why the branching end up there. What I was concerned about was this commit (4759eb1) which seems to have happened prior to the Python 3.12 issues being identified. This is how greenlet entered the declared dependency closure. If we're highly confident this was a misidentified issue, we can test out of the docker image. For CI, yes, we need to fix the bare |
The magic words:
Closed this lol |
@@ -35,8 +35,6 @@ dependencies = [ | |||
"decorator", | |||
"flasgger", | |||
"flask >= 2.2.4", | |||
'greenlet < 3.0; python_version<"3.12"', |
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.
It looks like this was also incorrectly pinning to greenlet <3 on older pythons
@nateprewitt can you take a look at this? I'd like to get 3.13 support landed in vcrpy and pytest-httpbin before the final release of 3.13 |
I've got this on my list for review next week. We should be able to get a release out then. |
@nateprewitt greenlet now has support for python 3.13 so this is not critical - would still be useful for when 3.14 is in beta |
No description provided.