perf: defer the requests imports in poetry.utils.helpers

[dexhunter]

Summary

src/poetry/console/application.py imports two filesystem helpers from
poetry.utils.helpers:

from poetry.utils.helpers import directory
from poetry.utils.helpers import ensure_path

poetry/utils/helpers.py imports the HTTP stack at module level:

from requests.exceptions import ChunkedEncodingError
from requests.exceptions import ConnectionError
from requests.utils import atomic_open

from poetry.utils.authenticator import get_default_authenticator

All four are used only inside Downloader, but every poetry command pays for them at
startup, including ones that never open a socket. This moves them to their point of use
and keeps them importable from the module through a PEP 562 __getattr__.

Numbers

Cost of import poetry.console.application in a fresh interpreter, measured as the wall
time of python -B -c "import poetry.console.application" minus the wall time of
python -B -c "pass" in the same window, median of 9, on an otherwise idle core,
CPython 3.12.13:

	before	after
import cost	156.8 ms	82.3 ms

requests and urllib3 drop out of sys.modules entirely for commands that do not use
them, which is where the time goes.

Two caveats worth stating. The absolute number depends on what else is installed in the
environment: in a second virtualenv on the same machine requests alone costs 104 ms
rather than 36 ms, and the same change measured 317 ms to 117 ms there. The ratio is the
stable part, not the milliseconds. And this is process startup only; it does not make
any command's actual work faster.

For context on why startup is worth anything here, #10625 measured poetry --version at
about 217 ms and poetry --version --no-plugins at about 116 ms. That issue and #10992
are about the plugin-discovery half of that gap. This is the other half, the part that
remains with --no-plugins, and the two are independent.

Behaviour is unchanged

ChunkedEncodingError, ConnectionError, atomic_open and get_default_authenticator
remain importable from poetry.utils.helpers and resolve to the same objects. dir() on
the module is unchanged.

The part that needed care: a module-level __getattr__ serves attribute access on the
module object, but never a bare global name lookup inside the module's own functions.
So writing

except (ChunkedEncodingError, ConnectionError):

with ConnectionError supplied lazily would silently bind the builtin
ConnectionError. requests.exceptions.ConnectionError is not a subclass of it, so
resumable downloads would stop retrying and turn into hard failures, and nothing in the
existing test suite would notice. The retry site imports both names into function scope
instead, where the local binding shadows the builtin correctly.

Tests

tests/utils/test_helpers_lazy_imports.py, 11 tests:

• each of the four names is still importable, is in dir(), and is the same object
• an unknown attribute still raises AttributeError
• importing poetry.console.application leaves requests and urllib3 out of
  sys.modules - this is the regression the change exists to prevent, and it fails on
  current main
• Downloader still retries ConnectionError, ChunkedEncodingError and SSLError
  (the last subclasses requests' ConnectionError, so it is caught by the same clause,
  and only if that name is the requests class), and still propagates ValueError and
  AttributeError rather than swallowing them in a broad except

I checked these fail when they should. On unmodified main the import test fails. On a
variant that lets ConnectionError fall through to the builtin, and on one that widens
the clause to except Exception, the retry test fails.

tests/utils gives 358 passed / 23 failed / 21 skipped; the same 23 fail by name on an
unmodified checkout of this base and need Python interpreter managers this environment
does not have. mypy reports no issues on the changed file. ruff check is clean on
both files, apart from one pre-existing PLC0206 in merge_dicts that is present on
main too and is untouched here.

Notes

I tried a number of shapes for this, each scored against the unmodified module for
identical public surface and identical retry behaviour. The record is at
https://dashboard.weco.ai/share/kStcipB3sa9sf-YTFM_fv0G6_ZyLa61H

The larger variants also defer logging, hashlib, tarfile, zipfile and the
module-level prioritised_hash_types tables. They measured between 78.8 ms and 81.7 ms,
a 2.9 ms span against a 3.8 ms run-to-run noise band, so they are not distinguishable
from this one. They are three times the diff for no measurable gain, and they move
module-level public attributes behind a hook, so I kept the smaller change. Happy to
push the broader version if you would prefer it.

Pull Request Check List

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

[sourcery-ai]

Hey - I've found 1 issue

Prompt for AI Agents

Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="tests/utils/test_helpers_lazy_imports.py" line_range="65-74" />
<code_context>
+def test_downloader_retries_only_the_resumable_requests_errors() -> None:
</code_context>
<issue_to_address>
**suggestion (testing):** Consider an additional test for `Downloader` when retry limit is exceeded to guard the non-resumable path

This test already covers resumable errors and non-resumable exceptions well. Please also add a case where resumable failures exceed `max_retries` (e.g. the stub raises `requests.exceptions.ConnectionError` on every call) and assert that, after the configured number of retries, the exception is propagated. This will confirm the lazy import refactor preserved the retry limit behaviour and that resumable errors are not retried indefinitely.

Suggested implementation:

```python
    out = subprocess.run(
        [sys.executable, "-c", code], capture_output=True, text=True, check=True
    ).stdout.strip()
    assert out == "False False", f"expected neither to be imported, got {out}"


def test_downloader_stops_retrying_after_max_retries(tmp_path, monkeypatch) -> None:
    """
    Ensure resumable failures are not retried indefinitely and that the configured
    retry limit is respected: once the maximum number of retries is exceeded,
    the last requests.exceptions.ConnectionError is propagated.
    """
    import requests.exceptions

    # import the Downloader from the helpers module under test
    from poetry.utils.helpers import Downloader  # adjust import to match project layout

    # choose a small max_retries value to keep the test fast and deterministic
    max_retries = 3

    # create a destination path in the tmp directory
    dest = tmp_path / "downloaded-file"

    call_count = {"value": 0}

    def always_failing_get(*args, **kwargs):
        # track how many times the underlying request function is called
        call_count["value"] += 1
        # emulate a resumable requests error on every attempt
        raise requests.exceptions.ConnectionError("synthetic connection error")

    # patch the low-level requests API used by Downloader so that every call fails
    import requests

    monkeypatch.setattr(requests, "get", always_failing_get)

    downloader = Downloader(max_retries=max_retries)

    # The Downloader should try up to max_retries times and then propagate the
    # last ConnectionError instead of retrying indefinitely.
    try:
        downloader.download("https://example.invalid/resource", dest)
    except requests.exceptions.ConnectionError as exc:
        # ensure we actually attempted the configured number of retries
        assert call_count["value"] == max_retries
        # the propagated exception should be the same type we raised
        assert isinstance(exc, requests.exceptions.ConnectionError)
    else:
        # if no exception was raised, the retry limit behaviour is broken
        raise AssertionError(
            "Downloader.download() did not propagate ConnectionError "
            f"after {max_retries} resumable failures"
        )


def test_downloader_retries_only_the_resumable_requests_errors() -> None:
    """`except ConnectionError` inside this module must be the requests class, not the
    builtin. A module-level ``__getattr__`` serves attribute access on the module object
    but never bare global lookup inside the module's own functions, so a lazy rewrite can
    silently bind the builtin -- which requests' ConnectionError is not a subclass of --
    and stop retrying resumable downloads."""
    import requests.exceptions

    def run_with(exc: type[BaseException], tmp_path_factory: object = None) -> int:
        import tempfile


```

The new test assumes the following about your existing `Downloader` implementation and test setup; please align these details with the actual code:

1. **Downloader import and constructor**  
   - Adjust `from poetry.utils.helpers import Downloader` to import `Downloader` from the correct module under test (e.g. `from poetry.utils.helpers_lazy_imports import Downloader` or wherever it actually lives).  
   - If `Downloader` does not take `max_retries` as a constructor argument, pass the retry limit using the real API (for example, `Downloader(retries=max_retries)`, `Downloader(session, max_retries=max_retries)`, or via configuration on `download()`).

2. **Download method signature**  
   - The test uses `downloader.download(url, dest)`. If your real method has a different signature (e.g. `download(url, dest, checksum=None)` or returns a value), update the call and assertions accordingly while keeping the behaviour-under-test: it should perform `max_retries` attempts and then raise `requests.exceptions.ConnectionError`.

3. **Requests patching location**  
   - The test currently patches `requests.get`. If `Downloader` uses a different entry point (e.g. `requests.Session.get`, `requests.request`, or `urllib3`), update the `monkeypatch.setattr(...)` target to match the actual call used in the resumable path, so all attempts hit the failing stub.

4. **Fixtures and typing**  
   - The test uses `tmp_path` and `monkeypatch` as standard pytest fixtures. Ensure your test file is configured to use these fixtures (they are built-in in pytest).  
   - If you use type hints for fixtures, you can annotate them (e.g. `tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch`) and add any necessary imports (`import pathlib`, `import pytest`) at the top of the file.

Once these adjustments are made to reflect your actual `Downloader` implementation, this test will verify that resumable errors respect the configured retry limit and are not retried indefinitely, preserving the intended behaviour after the lazy import refactor.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[radoering]

I gave it a quick try and it seems to be worth it. However, I wonder whether we could extract download_file and Downloader into a separate download module and keep the imports in this module global.

We can omit the reexports of requests and authenticator in helpers since this should not be used by anybody. However, when moving download_file and Downloader into a separate module, we may want to apply this pattern in combination with a DeprecationWarning for download_file and Downloader because they might be used in plugins.

Please let me know if you want to give this proposal a try. Otherwise, I may have a look for myself.

[dexhunter]

Thanks, I gave this a try.

I moved Downloader and download_file into poetry.utils.download, kept the requests and authenticator imports global in that module, and changed Poetry's internal callers to import from it directly. poetry.utils.helpers now keeps deprecated compatibility reexports for the two public helpers.

I also added coverage for retry exhaustion: with max_retries=2, the downloader consumes the initial response plus two resumed responses, then propagates the final connection error.

Validation on this exact commit: 60 focused tests passed; the broader affected set passed 182 tests with 1 skip; mypy and targeted pre-commit checks are clean. The paired import measurement moved from 149.85 ms to 78.08 ms.