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

fix(deps): bump requests-toolbelt to fix deprecation warning #2408

Merged
merged 1 commit into from Dec 5, 2022

Conversation

nejch
Copy link
Member

@nejch nejch commented Dec 4, 2022

Closes #2263

@lmilbaum
Copy link
Collaborator

lmilbaum commented Dec 4, 2022

@nejch Does it make sense to add a test that verifies the warning is no longer present?

@nejch
Copy link
Member Author

nejch commented Dec 4, 2022

@nejch Does it make sense to add a test that verifies the warning is no longer present?

@lmilbaum the warning was reported here due to upstream inactivity, but actually coming from a transitive dependency (urllib3) via another dependency (requests-toolbelt). It was fixed in that release, so I think a test should be upstream in toolbelt rather than in the scope of our library (maybe this was already done in requests/toolbelt#336). Unless I misunderstood and you meant a more general test for deprecation warnings (not sure if that's feasible, deprecation warnings do happen).

In any case this is a very minor issue, we do not restrict a maximum version of requests-toolbelt, so users are free to upgrade already on their own (and if they don't pin transitive dependencies in lockfiles then they probably are already on the latest version). Hope that makes sense :)

@lmilbaum
Copy link
Collaborator

lmilbaum commented Dec 5, 2022

@nejch Does it make sense to add a test that verifies the warning is no longer present?

@lmilbaum the warning was reported here due to upstream inactivity, but actually coming from a transitive dependency (urllib3) via another dependency (requests-toolbelt). It was fixed in that release, so I think a test should be upstream in toolbelt rather than in the scope of our library (maybe this was already done in requests/toolbelt#336). Unless I misunderstood and you meant a more general test for deprecation warnings (not sure if that's feasible, deprecation warnings do happen).

In any case this is a very minor issue, we do not restrict a maximum version of requests-toolbelt, so users are free to upgrade already on their own (and if they don't pin transitive dependencies in lockfiles then they probably are already on the latest version). Hope that makes sense :)

Totally makes sense. Thanks for the clarification.

@JohnVillalovos JohnVillalovos merged commit faf842e into main Dec 5, 2022
@JohnVillalovos JohnVillalovos deleted the fix/requests-toolbelt-deprecation-warning branch December 5, 2022 06:53
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.

Deprecation warning due to requests-toolbelt dependency
3 participants