Skip to content

Commit

Permalink
add timeouts to all requests (#5881)
Browse files Browse the repository at this point in the history
* add timeouts to all requests
* make the requests timeout a constant
* 15s (matching the default in pip)
  • Loading branch information
dimbleby committed Jun 19, 2022
1 parent a3aafa8 commit 584aaee
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 7 deletions.
3 changes: 3 additions & 0 deletions src/poetry/publishing/uploader.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from urllib3 import util

from poetry.__version__ import __version__
from poetry.utils.constants import REQUESTS_TIMEOUT
from poetry.utils.patterns import wheel_file_re


Expand Down Expand Up @@ -262,6 +263,7 @@ def _upload_file(
data=monitor,
allow_redirects=False,
headers={"Content-Type": monitor.content_type},
timeout=REQUESTS_TIMEOUT,
)
if resp is None or 200 <= resp.status_code < 300:
bar.set_format(
Expand Down Expand Up @@ -320,6 +322,7 @@ def _register(self, session: requests.Session, url: str) -> requests.Response:
data=encoder,
allow_redirects=False,
headers={"Content-Type": encoder.content_type},
timeout=REQUESTS_TIMEOUT,
)

resp.raise_for_status()
Expand Down
5 changes: 4 additions & 1 deletion src/poetry/repositories/http.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from poetry.repositories.exceptions import RepositoryError
from poetry.repositories.link_sources.html import HTMLPage
from poetry.utils.authenticator import Authenticator
from poetry.utils.constants import REQUESTS_TIMEOUT
from poetry.utils.helpers import download_file
from poetry.utils.patterns import wheel_file_re

Expand Down Expand Up @@ -260,7 +261,9 @@ def _links_to_data(self, links: list[Link], data: PackageInfo) -> dict[str, Any]
def _get_response(self, endpoint: str) -> requests.Response | None:
url = self._url + endpoint
try:
response: requests.Response = self.session.get(url, raise_for_status=False)
response: requests.Response = self.session.get(
url, raise_for_status=False, timeout=REQUESTS_TIMEOUT
)
if response.status_code in (401, 403):
self._log(
f"Authorization error accessing {url}",
Expand Down
13 changes: 10 additions & 3 deletions src/poetry/repositories/pypi_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from poetry.repositories.exceptions import PackageNotFound
from poetry.repositories.http import HTTPRepository
from poetry.utils._compat import to_str
from poetry.utils.constants import REQUESTS_TIMEOUT


cache_control_logger.setLevel(logging.ERROR)
Expand Down Expand Up @@ -103,7 +104,9 @@ def search(self, query: str) -> list[Package]:

search = {"q": query}

response = requests.session().get(self._base_url + "search", params=search)
response = requests.session().get(
self._base_url + "search", params=search, timeout=REQUESTS_TIMEOUT
)
content = parse(response.content, namespaceHTMLElements=False)
for result in content.findall(".//*[@class='package-snippet']"):
name_element = result.find("h3/*[@class='package-snippet__name']")
Expand Down Expand Up @@ -244,14 +247,18 @@ def _get_release_info(
def _get(self, endpoint: str) -> dict[str, Any] | None:
try:
json_response = self.session.get(
self._base_url + endpoint, raise_for_status=False
self._base_url + endpoint,
raise_for_status=False,
timeout=REQUESTS_TIMEOUT,
)
except requests.exceptions.TooManyRedirects:
# Cache control redirect loop.
# We try to remove the cache and try again
self.session.delete_cache(self._base_url + endpoint)
json_response = self.session.get(
self._base_url + endpoint, raise_for_status=False
self._base_url + endpoint,
raise_for_status=False,
timeout=REQUESTS_TIMEOUT,
)

if json_response.status_code != 200:
Expand Down
3 changes: 2 additions & 1 deletion src/poetry/utils/authenticator.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

from poetry.config.config import Config
from poetry.exceptions import PoetryException
from poetry.utils.constants import REQUESTS_TIMEOUT
from poetry.utils.password_manager import HTTPAuthCredential
from poetry.utils.password_manager import PasswordManager

Expand Down Expand Up @@ -219,7 +220,7 @@ def request(

# Send the request.
send_kwargs = {
"timeout": kwargs.get("timeout"),
"timeout": kwargs.get("timeout", REQUESTS_TIMEOUT),
"allow_redirects": kwargs.get("allow_redirects", True),
}
send_kwargs.update(settings)
Expand Down
5 changes: 5 additions & 0 deletions src/poetry/utils/constants.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from __future__ import annotations


# Timeout for HTTP requests using the requests library.
REQUESTS_TIMEOUT = 15
4 changes: 3 additions & 1 deletion src/poetry/utils/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
from typing import Iterator
from typing import Mapping

from poetry.utils.constants import REQUESTS_TIMEOUT


if TYPE_CHECKING:
from collections.abc import Callable
Expand Down Expand Up @@ -89,7 +91,7 @@ def download_file(

get = requests.get if not session else session.get

response = get(url, stream=True)
response = get(url, stream=True, timeout=REQUESTS_TIMEOUT)
response.raise_for_status()

set_indicator = False
Expand Down
4 changes: 3 additions & 1 deletion tests/repositories/test_legacy_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,9 @@ def test_get_redirected_response_url(
repo = MockHttpRepository({"/foo": 200}, http)
redirect_url = "http://legacy.redirect.bar"

def get_mock(url: str, raise_for_status: bool = True) -> requests.Response:
def get_mock(
url: str, raise_for_status: bool = True, timeout: int = 5
) -> requests.Response:
response = requests.Response()
response.status_code = 200
response.url = redirect_url + "/foo"
Expand Down

1 comment on commit 584aaee

@Kaiser1989
Copy link

Choose a reason for hiding this comment

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

Finally found it, 15s is way too short. We need to be able to configure this.

Please sign in to comment.