-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Switch our requests pin to only exclude 2.32.0 #21996
Conversation
Both docker-py and requests have fixes in flight. We can narrow our pin to only exclude the breaking version. docker/docker-py#3257 psf/requests#6707 (comment)
python_modules/dagster/setup.py
Outdated
@@ -138,7 +138,7 @@ def get_version() -> str: | |||
"morefs[asynclocal]", | |||
"fsspec<2024.5.0", # morefs incompatibly | |||
"rapidfuzz", | |||
"requests<2.32.0", # 2.32.0 breaks our docker tests https://buildkite.com/dagster/dagster-dagster/builds/83562 | |||
"requests!=2.32.0", # https://github.com/dagster-io/dagster/pull/21977 |
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.
do we need it here at all with the pin in place in dagster-docker
?
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.
[tests]
installs docker
directly
requests had a second patch release yesterday where the issue still persists.
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.
not clear to me this needs to be anywhere other than dagster-docker given the stack trace? and maybe dagster[test]?
We're just going to revert once docker publishes to pypi. |
Both docker-py and requests have fixes in flight. We can narrow our pin to only exclude the breaking version.
docker/docker-py#3257
psf/requests#6707 (comment)