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

feat: update cachecontrol to 0.13.0 #8055

Merged
merged 3 commits into from
Jun 2, 2023

Conversation

ralbertazzi
Copy link
Contributor

Pull Request Check List

Closes: #7997

@Secrus didn't want to step on your toes, just provide help :)

@popadi
Copy link

popadi commented Jun 2, 2023

Initially I hoped that a new cachecontrol release will silently fix the issue described here (without having to update to a newer version of poetry OR to pin urllib3). However, because the toml file has ^0.12.9 for all releases and because the new cachecontrol was release as 0.13.0, this won't happen.

Question for the maintainers: what will be the lucky poetry versions that will benefit from this?

@ralbertazzi
Copy link
Contributor Author

without having to update to a newer version of poetry OR to pin urllib3

I don't think anyone will release a fix for older versions of Poetry due to the limited time that maintainers can invest. The issue has been closed since fixed in latest versions, please update to 1.5

@dimbleby
Copy link
Contributor

dimbleby commented Jun 2, 2023

recent versions of poetry all pin to urrlib3<2, therefore if correctly installed they are not exposed to that issue anyway.

however this reminds me that this MR should allow that constraint to be relaxed, urllib3 version 3 is now out.

@ralbertazzi
Copy link
Contributor Author

Could you point me to the release? I can only see 2.0.2

@dimbleby
Copy link
Contributor

dimbleby commented Jun 2, 2023

yes sorry, I meant 2. but either way, this MR should allow that to be relaxed

@@ -60,7 +58,6 @@ tomlkit = ">=0.11.4,<1.0.0"
trove-classifiers = ">=2022.5.19"
virtualenv = "^20.22.0"
xattr = { version = "^0.10.0", markers = "sys_platform == 'darwin'" }
urllib3 = "^1.26.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

poetry still has a direct dependency on urllib3 -

from urllib3 import util
- so this should not be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seemed really minor for me to be kept but I understand the reasoning, better to be safe than sorry!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you prefer to relax the constraint or pin it to ^2.0.2 to enforce the usage of a more recent version? I'd prefer the second but I also understand the Poetry direct dependency on urllib3 works ok with <2 too..

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so far as I can see that import is only used to set up a Retry, which is configured to retry only for GETs; and the uploader never does a GET, it only POSTs.

So if that's right then that little pile of code could be deleted anyway. But double-check, I only skimmed it..!

Copy link
Contributor Author

@ralbertazzi ralbertazzi Jun 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are totally right. Wouldn't it be better to then fix it to "POST" if the original goal was to retry uploads?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meh, if the code has been there for this long without anyone feeling the need to have working retries - I'd just delete it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was just writing that 😄 Makes sense

@Secrus Secrus merged commit fca3d5d into python-poetry:master Jun 2, 2023
@frostming
Copy link
Contributor

However, because the toml file has ^0.12.9 for all releases and because the new cachecontrol was release as 0.13.0, this won't happen.

I've released v0.12.13 with the necessary fixes

@ralbertazzi ralbertazzi deleted the feat/cachecontrol-0-13 branch June 5, 2023 07:34
Copy link

github-actions bot commented Mar 3, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants