Skip to content

Comments

After initial fetch, save potentially-redirected url#9040

Closed
thatch wants to merge 1 commit intopython-poetry:mainfrom
thatch:lazy-redirects
Closed

After initial fetch, save potentially-redirected url#9040
thatch wants to merge 1 commit intopython-poetry:mainfrom
thatch:lazy-redirects

Conversation

@thatch
Copy link
Contributor

@thatch thatch commented Feb 26, 2024

Pull Request Check List

This is a draft -- no tests or relnotes. But appears to fix the issue in our environment.

Resolves: #9039

  • Added tests for changed code.
  • Updated documentation for changed code.

@radoering
Copy link
Member

Regarding tests: The only meaningful test I can imagine is a network test like

@pytest.mark.network
def test_metadata_from_wheel_url_with_redirect() -> None:
    url = (
        "https://httpbin.org/redirect-to?url=https://files.pythonhosted.org"
        "/packages/2d/99/6b0c5fe90e247b2b7b96a27cdf39ee59a02aab3c01d7243fc0c63cd7fb73"
        "/poetry_core-1.5.0-py3-none-any.whl"
    )
    metadata = metadata_from_wheel_url("poetry-core", url, requests.Session())

    assert metadata["name"] == "poetry-core"
    assert metadata["version"] == "1.5.0"
    assert metadata["author"] == "Sébastien Eustace"
    assert metadata["requires_dist"] == [
        'importlib-metadata (>=1.7.0) ; python_version < "3.8"'
    ]

This takes 1-2 seconds. Not sure if we want that or rather do without a test.

@thatch thatch force-pushed the lazy-redirects branch 2 times, most recently from c515344 to 95c3981 Compare February 27, 2024 18:01
@abn
Copy link
Member

abn commented Feb 28, 2024

@thatch please run the following locally to get the failing pre-commit job passing.

poetry run pre-commit install
poetry run pre-commit run --all

@abn
Copy link
Member

abn commented Feb 28, 2024

Regarding tests: The only meaningful test I can imagine is a network test like

@pytest.mark.network
def test_metadata_from_wheel_url_with_redirect() -> None:
    url = (
        "https://httpbin.org/redirect-to?url=https://files.pythonhosted.org"
        "/packages/2d/99/6b0c5fe90e247b2b7b96a27cdf39ee59a02aab3c01d7243fc0c63cd7fb73"
        "/poetry_core-1.5.0-py3-none-any.whl"
    )
    metadata = metadata_from_wheel_url("poetry-core", url, requests.Session())

    assert metadata["name"] == "poetry-core"
    assert metadata["version"] == "1.5.0"
    assert metadata["author"] == "Sébastien Eustace"
    assert metadata["requires_dist"] == [
        'importlib-metadata (>=1.7.0) ; python_version < "3.8"'
    ]

This takes 1-2 seconds. Not sure if we want that or rather do without a test.

We can modify the test at https://github.com/python-poetry/poetry/pull/9051/files#diff-d86ccfe236884d03286aee144a8a8dcb2016aa497261e05d81da2a7a1101ade6R406-R419 to account for this.

def test_metadata_from_wheel_url_with_redirect_after_500(
    assert_metadata_from_wheel_url: AssertMetadataFromWheelUrl,
) -> None:
    assert_metadata_from_wheel_url(
        negative_offset_error=(codes.internal_server_error, b"Internal server error"),
        expected_requests=10,
        redirect=True,
    )

@github-actions
Copy link

github-actions bot commented Apr 2, 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 Apr 2, 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.

Lazy wheel broken in 1.8.0 and 1.8.1 when there are redirects

4 participants