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: consistently retry on error codes and respect Retry-After header #7072

Merged
merged 2 commits into from
Apr 4, 2023

Conversation

nejch
Copy link
Contributor

@nejch nejch commented Nov 22, 2022

Closes #7679.

This applies a common list of status codes to retry between poetry publish and poetry install . Previously, 500, 501, 502, 503 was used for publish and 502, 503, 504 for install.

Additionally, 02ffff8 adds support for the Retry-After header to properly handle 429 responses (https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/429).

Note: this does not touch the subprocess pip install commands, I haven't checked how pip handles retries.

Edit: marking this as ready even though there are some failures as I can't reproduce them locally, in case anyone has ideas already. Will try to check once I can get to a Windows machine.

Pull Request Check List

  • Added tests for changed code.
  • Updated documentation for changed code. (Not applicable from what I can see)

@nejch nejch force-pushed the fix/consistent-retries branch from 02ffff8 to 55a84a3 Compare November 22, 2022 15:40
@nejch nejch marked this pull request as ready for review November 22, 2022 16:20
@nejch nejch force-pushed the fix/consistent-retries branch from 55a84a3 to c8e7b71 Compare December 7, 2022 08:36
@nejch nejch force-pushed the fix/consistent-retries branch 2 times, most recently from 1075244 to c8bbb27 Compare April 1, 2023 10:14
@nejch
Copy link
Contributor Author

nejch commented Apr 1, 2023

Alright. I finally got to a Windows machine today and fixed the test. This should be ready for review.

/cc @novas0x2a @dimbleby you were active in the related issue, if you have any tips on moving forward here, just let me know ;) not sure if it this hasn't been reviewed because of the CI failure or more general maintainer shortage.

@nejch nejch force-pushed the fix/consistent-retries branch from c8bbb27 to 92933fd Compare April 2, 2023 06:36
@dimbleby
Copy link
Contributor

dimbleby commented Apr 2, 2023

if I knew how to persuade folk to hit the merge button I would gladly share that secret!

@nejch
Copy link
Contributor Author

nejch commented Apr 2, 2023

if I knew how to persuade folk to hit the merge button I would gladly share that secret!

No worries, people have their own lives and priorities, just wanted to check if some of you more regular contributors might have a flow that works well ;)

@radoering
Copy link
Member

Due to the limited time, I only take a closer look at a PR I'm not personally interested in if the PR looks good at first glance and/or seems to solve an issue that hits many users.

Some positive criteria:

  • pipelines do not fail
  • test that fails without the change was added
  • change does not conflict with main branch
  • a regular contributor (such as dimbleby) reviewed/approved the PR
  • users claim they have tested the change and it fixes the issue for them

Even if you created the perfect PR, you can be unlucky if all maintainers are busy with other things at the time. In that case it might make sense to ask on our Discord server after a month or so.

@nejch nejch force-pushed the fix/consistent-retries branch 2 times, most recently from 6a57c2b to c776541 Compare April 3, 2023 12:35
@nejch
Copy link
Contributor Author

nejch commented Apr 3, 2023

Due to the limited time, I only take a closer look at a PR I'm not personally interested in if the PR looks good at first glance and/or seems to solve an issue that hits many users.

Some positive criteria:

  • pipelines do not fail
  • test that fails without the change was added
  • change does not conflict with main branch
  • a regular contributor (such as dimbleby) reviewed/approved the PR
  • users claim they have tested the change and it fixes the issue for them

Even if you created the perfect PR, you can be unlucky if all maintainers are busy with other things at the time. In that case it might make sense to ask on our Discord server after a month or so.

Thanks for taking a look @radoering! Should be ready for another round now, I've left the cloudfare codes out so I can look mocking custom codes in httpretty separately.

As this is mostly dealing with transient errors, it might be a bit harder for users to reproduce the condition reliably, but if any users here see this PR/comment, and would like to give it a shot, just chime in and let us know 🙇

@nejch nejch requested a review from radoering April 3, 2023 12:55
@radoering radoering merged commit 6b3a616 into python-poetry:master Apr 4, 2023
@nejch nejch deleted the fix/consistent-retries branch April 4, 2023 08:32
mwalbeck pushed a commit to mwalbeck/docker-python-poetry that referenced this pull request May 23, 2023
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [poetry](https://python-poetry.org/) ([source](https://github.com/python-poetry/poetry), [changelog](https://python-poetry.org/history/)) | minor | `1.4.2` -> `1.5.0` |

---

### Release Notes

<details>
<summary>python-poetry/poetry</summary>

### [`v1.5.0`](https://github.com/python-poetry/poetry/blob/HEAD/CHANGELOG.md#&#8203;150---2023-05-19)

[Compare Source](python-poetry/poetry@1.4.2...1.5.0)

##### Added

-   **Introduce the new source priorities `explicit` and `supplemental`** ([#&#8203;7658](python-poetry/poetry#7658),
    [#&#8203;6879](python-poetry/poetry#6879)).
-   **Introduce the option to configure the priority of the implicit PyPI source** ([#&#8203;7801](python-poetry/poetry#7801)).
-   Add handling for corrupt cache files ([#&#8203;7453](python-poetry/poetry#7453)).
-   Improve caching of URL and git dependencies ([#&#8203;7693](python-poetry/poetry#7693),
    [#&#8203;7473](python-poetry/poetry#7473)).
-   Add option to skip installing directory dependencies ([#&#8203;6845](python-poetry/poetry#6845),
    [#&#8203;7923](python-poetry/poetry#7923)).
-   Add `--executable` option to `poetry env info` ([#&#8203;7547](python-poetry/poetry#7547)).
-   Add `--top-level` option to `poetry show` ([#&#8203;7415](python-poetry/poetry#7415)).
-   Add `--lock` option to `poetry remove` ([#&#8203;7917](python-poetry/poetry#7917)).
-   Add experimental `POETRY_REQUESTS_TIMEOUT` option ([#&#8203;7081](python-poetry/poetry#7081)).
-   Improve performance of wheel inspection by avoiding unnecessary file copy operations ([#&#8203;7916](python-poetry/poetry#7916)).

##### Changed

-   **Remove the old deprecated installer and the corresponding setting `experimental.new-installer`** ([#&#8203;7356](python-poetry/poetry#7356)).
-   **Introduce `priority` key for sources and deprecate flags `default` and `secondary`** ([#&#8203;7658](python-poetry/poetry#7658)).
-   Deprecate `poetry run <entry point>` if the entry point was not previously installed via `poetry install` ([#&#8203;7606](python-poetry/poetry#7606)).
-   Only write the lock file if the installation succeeds ([#&#8203;7498](python-poetry/poetry#7498)).
-   Do not write the unused package category into the lock file ([#&#8203;7637](python-poetry/poetry#7637)).

##### Fixed

-   Fix an issue where Poetry's internal pyproject.toml continually grows larger with empty lines ([#&#8203;7705](python-poetry/poetry#7705)).
-   Fix an issue where Poetry crashes due to corrupt cache files ([#&#8203;7453](python-poetry/poetry#7453)).
-   Fix an issue where the `Retry-After` in HTTP responses was not respected and retries were handled inconsistently ([#&#8203;7072](python-poetry/poetry#7072)).
-   Fix an issue where Poetry silently ignored invalid groups ([#&#8203;7529](python-poetry/poetry#7529)).
-   Fix an issue where Poetry does not find a compatible Python version if not given explicitly ([#&#8203;7771](python-poetry/poetry#7771)).
-   Fix an issue where the `direct_url.json` of an editable install from a git dependency was invalid ([#&#8203;7473](python-poetry/poetry#7473)).
-   Fix an issue where error messages from build backends were not decoded correctly ([#&#8203;7781](python-poetry/poetry#7781)).
-   Fix an infinite loop when adding certain dependencies ([#&#8203;7405](python-poetry/poetry#7405)).
-   Fix an issue where pre-commit hooks skip pyproject.toml files in subdirectories ([#&#8203;7239](python-poetry/poetry#7239)).
-   Fix an issue where pre-commit hooks do not use the expected Python version ([#&#8203;6989](python-poetry/poetry#6989)).
-   Fix an issue where an unclear error message is printed if the project name is the same as one of its dependencies ([#&#8203;7757](python-poetry/poetry#7757)).
-   Fix an issue where `poetry install` returns a zero exit status even though the build script failed ([#&#8203;7812](python-poetry/poetry#7812)).
-   Fix an issue where an existing `.venv` was not used if `in-project` was not set ([#&#8203;7792](python-poetry/poetry#7792)).
-   Fix an issue where multiple extras passed to `poetry add` were not parsed correctly ([#&#8203;7836](python-poetry/poetry#7836)).
-   Fix an issue where `poetry shell` did not send a newline to `fish` ([#&#8203;7884](python-poetry/poetry#7884)).
-   Fix an issue where `poetry update --lock` printed operations that were not executed ([#&#8203;7915](python-poetry/poetry#7915)).
-   Fix an issue where `poetry add --lock` did perform a full update of all dependencies ([#&#8203;7920](python-poetry/poetry#7920)).
-   Fix an issue where `poetry shell` did not work with `nushell` ([#&#8203;7919](python-poetry/poetry#7919)).
-   Fix an issue where subprocess calls failed on Python 3.7 ([#&#8203;7932](python-poetry/poetry#7932)).
-   Fix an issue where keyring was called even though the password was stored in an environment variable ([#&#8203;7928](python-poetry/poetry#7928)).

##### Docs

-   Add information about what to use instead of `--dev` ([#&#8203;7647](python-poetry/poetry#7647)).
-   Promote semantic versioning less aggressively ([#&#8203;7517](python-poetry/poetry#7517)).
-   Explain Poetry's own versioning scheme in the FAQ ([#&#8203;7517](python-poetry/poetry#7517)).
-   Update documentation for configuration with environment variables ([#&#8203;6711](python-poetry/poetry#6711)).
-   Add details how to disable the virtualenv prompt ([#&#8203;7874](python-poetry/poetry#7874)).
-   Improve documentation on whether to commit `poetry.lock` ([#&#8203;7506](python-poetry/poetry#7506)).
-   Improve documentation of `virtualenv.create` ([#&#8203;7608](python-poetry/poetry#7608)).

##### poetry-core ([`1.6.0`](https://github.com/python-poetry/poetry-core/releases/tag/1.6.0))

-   Improve error message for invalid markers ([#&#8203;569](python-poetry/poetry-core#569)).
-   Increase robustness when deleting temporary directories on Windows ([#&#8203;460](python-poetry/poetry-core#460)).
-   Replace `tomlkit` with `tomli`, which changes the interface of some *internal* classes ([#&#8203;483](python-poetry/poetry-core#483)).
-   Deprecate `Package.category` ([#&#8203;561](python-poetry/poetry-core#561)).
-   Fix a performance regression in marker handling ([#&#8203;568](python-poetry/poetry-core#568)).
-   Fix an issue where wildcard version constraints were not handled correctly ([#&#8203;402](python-poetry/poetry-core#402)).
-   Fix an issue where `poetry build` created duplicate Python classifiers if they were specified manually ([#&#8203;578](python-poetry/poetry-core#578)).
-   Fix an issue where local versions where not handled correctly ([#&#8203;579](python-poetry/poetry-core#579)).

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS44Mi4wIiwidXBkYXRlZEluVmVyIjoiMzUuODIuMCIsInRhcmdldEJyYW5jaCI6Im1hc3RlciJ9-->

Reviewed-on: https://git.walbeck.it/walbeck-it/docker-python-poetry/pulls/717
Co-authored-by: renovate-bot <bot@walbeck.it>
Co-committed-by: renovate-bot <bot@walbeck.it>
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.

consider retrying 500 errors on poetry install
3 participants