poetry icon indicating copy to clipboard operation
poetry copied to clipboard

Downloader support resume from connection reset

Open austinzh opened this issue 1 year ago • 2 comments

Pull Request Check List

Resolves: #3219

  • [x] Added tests for changed code.
  • [x] Updated documentation for changed code.

austinzh avatar May 17 '24 21:05 austinzh

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.

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 installer.max-retries? I think it's still valid name right? Or we add solver.max-retries ? @radoering

austinzh avatar Jun 12 '24 00:06 austinzh

But for the config, should we still use installer.max-retries? I think it's still valid name right? Or we add solver.max-retries ?

Good question. Thinking about it, installer.max-retries might have been a bit ambiguous from the start: We only retry the download not the complete installation. I think we should introduce a new section, maybe call it network or requests? (We already have a POETRY_REQUESTS_TIMEOUT, which is not yet available via config but only via environment variable; anyway, I think that one would fit into the same section.)

radoering avatar Jun 16 '24 11:06 radoering

Please reopen https://github.com/python-poetry/poetry/issues/3219 to close the duplicate of #9551

SmartManoj avatar Jul 15 '24 10:07 SmartManoj

Please reopen #3219 to close the duplicate of #9551

I do not think reopening #3219 makes sense because it is was closed a long time ago. Generally, we do not need an issue if we already have a PR. However, since some users only search issues, I think it is ok to keep #9551 open until this one is merged.

radoering avatar Jul 15 '24 15:07 radoering

Does this PR need any change?

SmartManoj avatar Jul 29 '24 08:07 SmartManoj

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.

radoering avatar Jul 31 '24 09:07 radoering

Deploy preview for website ready!

✅ Preview https://website-8c1zr2f5e-python-poetry.vercel.app

Built with commit f8db4720e085103db93160a92f6b724319fc9ded. This pull request is being automatically deployed with vercel-action

github-actions[bot] avatar Aug 17 '24 15:08 github-actions[bot]

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[bot] avatar Sep 18 '24 00:09 github-actions[bot]