-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Downloader support resume from connection reset #9422
Conversation
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.
download_file()
is used in some more places (HTTPRepository
, DirectOrigin
), which might not be relevant for installing but only for locking. Maybe, connection resets are not that relevant for locking. On the other side, it feels more consistent to also retry in this case. What do you think?
HTTPRepository
already has the config available, so it is easy to use the setting. For DirectOrigin
, we had to add another parameter.
|
||
**Environment Variable**: `POETRY_INSTALLER_MAX_RETRIES` | ||
|
||
*Introduced in 1.9.0* |
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.
Reminder to myself: The next version might be 2.0.0
.
Sure. I can add parameter to them, It's my first time working in this repo, I agree that even they are not much needed for retry it's better to make it all consistent. But for the config, should we still use |
Good question. Thinking about it, |
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.
Change config to "requests.max-retries"
Does this PR need any change? |
I think all my comments have been addressed but I still have to review the changes. I won't have time for the next two weeks but feel free to remind me end of August if I have not come back to this issue by then. |
Co-authored-by: Randy Döring <[email protected]>
Deploy preview for website ready! ✅ Preview Built with commit f8db472. |
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. |
Pull Request Check List
Resolves: #3219
Resolves: #9551