Skip to content

Delay parsing requests timeout environment variable#10970

Open
nightt5879 wants to merge 2 commits into
python-poetry:mainfrom
nightt5879:nightt5879/fix-requests-timeout-import
Open

Delay parsing requests timeout environment variable#10970
nightt5879 wants to merge 2 commits into
python-poetry:mainfrom
nightt5879:nightt5879/fix-requests-timeout-import

Conversation

@nightt5879

Copy link
Copy Markdown

Summary

  • Delay parsing POETRY_REQUESTS_TIMEOUT until an HTTP request actually needs the timeout value.
  • Keep the default timeout constant import-safe and parse the environment value through Poetry's integer normalizer helper.
  • Add regression coverage for python -m poetry --version with an invalid timeout environment value.

Fixes #10969

Validation

  • POETRY_REQUESTS_TIMEOUT=abc .\.venv\Scripts\python.exe -m poetry --version -> Poetry (version 2.5.0.dev0)
  • .\.venv\Scripts\python.exe -m pytest -n 0 --basetemp=.pytest-tmp tests\utils\test_constants.py tests\console\test_application.py::test_application_version_ignores_invalid_requests_timeout -> 4 passed
  • .\.venv\Scripts\python.exe -m pytest -n 0 --basetemp=.pytest-tmp tests\utils\test_authenticator.py tests\repositories\test_http_repository.py tests\repositories\test_pypi_repository.py tests\publishing -> 173 passed
  • uvx ruff check src\poetry\utils\constants.py src\poetry\utils\authenticator.py src\poetry\utils\helpers.py src\poetry\repositories\http_repository.py src\poetry\repositories\pypi_repository.py src\poetry\publishing\uploader.py tests\console\test_application.py tests\utils\test_constants.py -> passed

Notes

  • Full pytest --basetemp=.pytest-tmp on this Windows/Python 3.13.13 environment reached 3183 passed / 11 skipped, with 10 existing environment-sensitive failures unrelated to this change. The failures involved Windows path URI expectations, active Python discovery preferring Anaconda Python, and missing embedded pip wheels from the locally resolved virtualenv package.

@nightt5879 nightt5879 marked this pull request as ready for review July 1, 2026 08:50

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • In PypiRepository._get and HTTPRepository._get_response the timeout argument has been removed from session.get, which makes those requests behave differently from others using get_requests_timeout() and may unintentionally rely on the default requests timeout—consider restoring an explicit timeout for consistency.
  • get_requests_timeout() re-reads and parses POETRY_REQUESTS_TIMEOUT from the environment on every call; if this is used frequently, consider caching the parsed value (including the invalid-case behavior) to avoid repeated env lookups and parsing.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `PypiRepository._get` and `HTTPRepository._get_response` the `timeout` argument has been removed from `session.get`, which makes those requests behave differently from others using `get_requests_timeout()` and may unintentionally rely on the default requests timeout—consider restoring an explicit timeout for consistency.
- `get_requests_timeout()` re-reads and parses `POETRY_REQUESTS_TIMEOUT` from the environment on every call; if this is used frequently, consider caching the parsed value (including the invalid-case behavior) to avoid repeated env lookups and parsing.

## Individual Comments

### Comment 1
<location path="src/poetry/repositories/http_repository.py" line_range="542-541" />
<code_context>
         url = self._url + endpoint
         try:
             response: requests.Response = self.session.get(
-                url, raise_for_status=False, timeout=REQUESTS_TIMEOUT, headers=headers
+                url, raise_for_status=False, headers=headers
             )
             if response.status_code in (401, 403):
</code_context>
<issue_to_address>
**issue (bug_risk):** Removing the timeout here may cause requests to hang indefinitely under network issues.

These calls were previously bounded by `REQUESTS_TIMEOUT`; without it, they now use the library default (`None`), so a slow or stalled connection can block indefinitely. If you’re standardizing timeouts, please either use `get_requests_timeout()` here or expose a configurable per-repository timeout instead of removing it completely.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/poetry/repositories/http_repository.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid POETRY_REQUESTS_TIMEOUT can fail at import time instead of using config normalization

1 participant