uploader: fix HTTP status code handling for redirects #7160

lineman60 · 2022-12-09T06:12:35Z

Pull Request Check List

Resolves: #7159

Added tests for changed code.
[N/A ] Updated documentation for changed code.

dimbleby · 2022-12-09T08:33:54Z

src/poetry/publishing/uploader.py

@@ -261,12 +261,12 @@ def _upload_file(
                        headers={"Content-Type": monitor.content_type},
                        timeout=REQUESTS_TIMEOUT,
                    )
-                if resp is None or 200 <= resp.status_code < 300:
+                if resp is None or 200 <= resp.status_code < 299:


you have introduced an out by one error here and elsewhere (what happens for 299? etc)

Thanks, fixed!

dimbleby · 2022-12-09T08:34:45Z

src/poetry/publishing/uploader.py

@@ -275,7 +275,9 @@ def _upload_file(
                        "Redirects are not supported. "
                        "Is the URL missing a trailing slash?"
                    )
-                elif resp.status_code == 400 and "was ever registered" in resp.text:
+                elif (


this code branch looks as though it was aimed at the specific behaviour of a particular server (almost surely pypi). I doubt that broadening the range of status codes is useful

hutson · 2022-12-09T16:57:20Z

src/poetry/publishing/uploader.py

                    bar.set_format(
                        f" - Uploading <c1>{file.name}</c1> <fg=green>%percent%%</>"
                    )
                    bar.finish()
-                elif resp.status_code == 301:
+                elif 300 <= resp.status_code < 399:


Suggested change

elif 300 <= resp.status_code < 399:

elif 300 <= resp.status_code < 400:

updated to 400

hutson · 2022-12-09T17:04:23Z

src/poetry/publishing/uploader.py

-                elif resp.status_code == 400 and "was ever registered" in resp.text:
+                elif (


Seems unrelated to addressing the behavior around 3xx codes - #7159

Removed this change.

dimbleby · 2022-12-10T15:06:31Z

This looks harmless to me, but I don't follow why this fix is necessary.

Wouldn't the code previously have gone through the else ... resp.raise_for_status() branch? and then through the except ... requests.HTTPError branch, raising an UploadError?

ie the command should have failed all along, just without the text about "Redirects are not supported..."?

Edit: no, because resp.raise_for_status() does not raise for 3XX status codes.

In which case, looks good to me.

tests/publishing/test_uploader.py

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.3.2` -> `1.4.0` | --- ### Release Notes <details> <summary>python-poetry/poetry</summary> ### [`v1.4.0`](https://github.com/python-poetry/poetry/blob/HEAD/CHANGELOG.md#140---2023-02-27) [Compare Source](python-poetry/poetry@1.3.2...1.4.0) ##### Added - **Add a modern installer (`installer.modern-installation`) for faster installation of packages and independence from pip** ([#6205](python-poetry/poetry#6205)). - Add support for `Private ::` trove classifiers ([#7271](python-poetry/poetry#7271)). - Add the version of poetry in the `@generated` comment at the beginning of the lock file ([#7339](python-poetry/poetry#7339)). - Add support for `virtualenvs.prefer-active-python` when running `poetry new` and `poetry init` ([#7100](python-poetry/poetry#7100)). ##### Changed - **Deprecate the old installer, i.e. setting `experimental.new-installer` to `false`** ([#7358](python-poetry/poetry#7358)). - Remove unused `platform` field from cached package info and bump the cache version ([#7304](python-poetry/poetry#7304)). - Extra dependencies of the root project are now sorted in the lock file ([#7375](python-poetry/poetry#7375)). - Remove upper boundary for `importlib-metadata` dependency ([#7434](python-poetry/poetry#7434)). - Validate path dependencies during use instead of during construction ([#6844](python-poetry/poetry#6844)). - Remove the deprecated `repository` modules ([#7468](python-poetry/poetry#7468)). ##### Fixed - Fix an issue where an unconditional dependency of an extra was not installed in specific environments ([#7175](python-poetry/poetry#7175)). - Fix an issue where a pre-release of a dependency was chosen even if a stable release fulfilled the constraint ([#7225](python-poetry/poetry#7225), [#7236](python-poetry/poetry#7236)). - Fix an issue where HTTP redirects were not handled correctly during publishing ([#7160](python-poetry/poetry#7160)). - Fix an issue where `poetry check` did not handle the `-C, --directory` option correctly ([#7241](python-poetry/poetry#7241)). - Fix an issue where the subdirectory information of a git dependency was not written to the lock file ([#7367](python-poetry/poetry#7367)). - Fix an issue where the wrong Python version was selected when creating an virtual environment ([#7221](python-poetry/poetry#7221)). - Fix an issue where packages that should be kept were uninstalled when calling `poetry install --sync` ([#7389](python-poetry/poetry#7389)). - Fix an issue where an incorrect value was set for `sys.argv[0]` when running installed scripts ([#6737](python-poetry/poetry#6737)). - Fix an issue where hashes in `direct_url.json` files were not written according to the specification ([#7475](python-poetry/poetry#7475)). - Fix an issue where poetry commands failed due to special characters in the path of the project or virtual environment ([#7471](python-poetry/poetry#7471)). - Fix an issue where poetry crashed with a `JSONDecodeError` when running a Python script that produced certain warnings ([#6665](python-poetry/poetry#6665)). ##### Docs - Add advice on how to maintain a poetry plugin ([#6977](python-poetry/poetry#6977)). - Update tox examples to comply with the latest tox release ([#7341](python-poetry/poetry#7341)). - Mention that the `poetry export` can export `constraints.txt` files ([#7383](python-poetry/poetry#7383)). - Add clarifications for moving configuration files ([#6864](python-poetry/poetry#6864)). - Mention the different types of exact version specifications ([#7503](python-poetry/poetry#7503)). ##### poetry-core ([`1.5.1`](https://github.com/python-poetry/poetry-core/releases/tag/1.5.1)) - Improve marker handling ([#528](python-poetry/poetry-core#528), [#534](python-poetry/poetry-core#534), [#530](python-poetry/poetry-core#530), [#546](python-poetry/poetry-core#546), [#547](python-poetry/poetry-core#547)). - Validate whether dependencies referenced in `extras` are defined in the main dependency group ([#542](python-poetry/poetry-core#542)). - Poetry no longer generates a `setup.py` file in sdists by default ([#318](python-poetry/poetry-core#318)). - Fix an issue where trailing newlines were allowed in `tool.poetry.description` ([#505](python-poetry/poetry-core#505)). - Fix an issue where the name of the data folder in wheels was not normalized ([#532](python-poetry/poetry-core#532)). - Fix an issue where the order of entries in the RECORD file was not deterministic ([#545](python-poetry/poetry-core#545)). - Fix an issue where zero padding was not correctly handled in version comparisons ([#540](python-poetry/poetry-core#540)). - Fix an issue where sdist builds did not support multiple READMEs ([#486](python-poetry/poetry-core#486)). ##### poetry-plugin-export ([`^1.3.0`](https://github.com/python-poetry/poetry-plugin-export/releases/tag/1.3.0)) - Fix an issue where the export failed if there was a circular dependency on the root package ([#118](python-poetry/poetry-plugin-export#118)). </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. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.walbeck.it/walbeck-it/docker-python-poetry/pulls/655 Co-authored-by: renovate-bot <[email protected]> Co-committed-by: renovate-bot <[email protected]>

github-actions · 2024-02-29T02:10:31Z

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.

lineman60 mentioned this pull request Dec 9, 2022

30X upload redirect bug, fails but reports success #7159

Closed

4 tasks

lineman60 force-pushed the fix/statusCode branch 2 times, most recently from 994a6a8 to e2bd78d Compare December 9, 2022 06:43

dimbleby reviewed Dec 9, 2022

View reviewed changes

hutson reviewed Dec 9, 2022

View reviewed changes

Fix: HTTP Upload status codes

3e3eb71

lineman60 force-pushed the fix/statusCode branch from e2bd78d to 3e3eb71 Compare December 10, 2022 01:31

lineman60 requested a review from dimbleby December 10, 2022 01:50

Merge branch 'master' into fix/statusCode

649a66e

Merge branch 'master' into fix/statusCode

be2970a

radoering reviewed Dec 19, 2022

View reviewed changes

tests/publishing/test_uploader.py Show resolved Hide resolved

lineman60 added 4 commits December 23, 2022 19:16

Merge branch 'master' into fix/statusCode

18d3b8e

Merge branch 'python-poetry:master' into fix/statusCode

be99a97

fix(test)

79fd3fd

fix white space

409d57d

radoering approved these changes Jan 15, 2023

View reviewed changes

radoering changed the title ~~Fix: HTTP Upload status codes~~ uploader: fix HTTP status code handling for redirects Jan 15, 2023

radoering merged commit a5c3846 into python-poetry:master Jan 15, 2023

neersighted added kind/bug Something isn't working as expected area/error-handling Bad error messages/insufficient error handling area/uploader Related to the repository uploader kind/enhancement Not a bug or feature, but improves usability or performance labels Jan 15, 2023

neersighted added this to the 1.4 milestone Jan 15, 2023

neersighted added the impact/changelog Requires a changelog entry label Jan 15, 2023

lineman60 deleted the fix/statusCode branch January 16, 2023 04:45

radoering mentioned this pull request Feb 18, 2023

release: bump version to 1.4.0 #7531

Merged

github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

uploader: fix HTTP status code handling for redirects #7160

uploader: fix HTTP status code handling for redirects #7160

lineman60 commented Dec 9, 2022 •

edited

Loading

dimbleby Dec 9, 2022

lineman60 Dec 10, 2022

dimbleby Dec 9, 2022

hutson Dec 9, 2022

lineman60 Dec 10, 2022

hutson Dec 9, 2022

lineman60 Dec 10, 2022

dimbleby commented Dec 10, 2022 •

edited

Loading

github-actions bot commented Feb 29, 2024

	elif 300 <= resp.status_code < 399:
	elif 300 <= resp.status_code < 400:

		elif resp.status_code == 400 and "was ever registered" in resp.text:
		elif (

uploader: fix HTTP status code handling for redirects #7160

uploader: fix HTTP status code handling for redirects #7160

Conversation

lineman60 commented Dec 9, 2022 • edited Loading

Pull Request Check List

dimbleby Dec 9, 2022

Choose a reason for hiding this comment

lineman60 Dec 10, 2022

Choose a reason for hiding this comment

dimbleby Dec 9, 2022

Choose a reason for hiding this comment

hutson Dec 9, 2022

Choose a reason for hiding this comment

lineman60 Dec 10, 2022

Choose a reason for hiding this comment

hutson Dec 9, 2022

Choose a reason for hiding this comment

lineman60 Dec 10, 2022

Choose a reason for hiding this comment

dimbleby commented Dec 10, 2022 • edited Loading

github-actions bot commented Feb 29, 2024

lineman60 commented Dec 9, 2022 •

edited

Loading

dimbleby commented Dec 10, 2022 •

edited

Loading