diff --git a/docs/configuration.rst b/docs/configuration.rst index 0e43b5c4..014b7bc4 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -158,7 +158,9 @@ Reporting to web service related settings. In most cases there is fallback to we * ``enabled`` - enable reporting to web service, boolean, optional, default ``yes`` for backward compatibility. * ``address`` - remote web service address with protocol, string, optional. In case of websocket, the ``http+unix`` scheme and URL encoded address (e.g. ``%2Fvar%2Flib%2Fahriman`` for ``/var/lib/ahriman``) must be used, e.g. ``http+unix://%2Fvar%2Flib%2Fahriman%2Fsocket``. In case if none set, it will be guessed from ``web`` section. +* ``max_retries`` - maximum amount of retries of HTTP requests, integer, optional, default ``0``. * ``password`` - password to authorize in web service in order to update service status, string, required in case if authorization enabled. +* ``retry_backoff`` - retry exponential backoff, float, optional, default ``0.0``. * ``suppress_http_log_errors`` - suppress HTTP log errors, boolean, optional, default ``no``. If set to ``yes``, any HTTP log errors (e.g. if web server is not available, but HTTP logging is enabled) will be suppressed. * ``timeout`` - HTTP request timeout in seconds, integer, optional, default is ``30``. * ``username`` - username to authorize in web service in order to update service status, string, required in case if authorization enabled. @@ -367,6 +369,8 @@ Section name must be either ``telegram`` (plus optional architecture name, e.g. * ``chat_id`` - telegram chat id, either string with ``@`` or integer value, required. * ``homepage`` - link to homepage, string, optional. * ``link_path`` - prefix for HTML links, string, required. +* ``max_retries`` - maximum amount of retries of HTTP requests, integer, optional, default ``0``. +* ``retry_backoff`` - retry exponential backoff, float, optional, default ``0.0``. * ``rss_url`` - link to RSS feed, string, optional. * ``template`` - Jinja2 template name, string, required. * ``template_type`` - ``parse_mode`` to be passed to telegram API, one of ``MarkdownV2``, ``HTML``, ``Markdown``, string, optional, default ``HTML``. @@ -392,6 +396,7 @@ Type will be read from several sources: This feature requires GitHub key creation (see below). Section name must be either ``github`` (plus optional architecture name, e.g. ``github:x86_64``) or random name with ``type`` set. * ``type`` - type of the upload, string, optional, must be set to ``github`` if exists. +* ``max_retries`` - maximum amount of retries of HTTP requests, integer, optional, default ``0``. * ``owner`` - GitHub repository owner, string, required. * ``password`` - created GitHub API key. In order to create it do the following: @@ -401,6 +406,7 @@ This feature requires GitHub key creation (see below). Section name must be eith #. Generate new token. Required scope is ``public_repo`` (or ``repo`` for private repository support). * ``repository`` - GitHub repository name, string, required. Repository must be created before any action and must have active branch (e.g. with readme). +* ``retry_backoff`` - retry exponential backoff, float, optional, default ``0.0``. * ``timeout`` - HTTP request timeout in seconds, integer, optional, default is ``30``. * ``use_full_release_name`` - if set to ``yes``, the release will contain both repository name and architecture, and only architecture otherwise, boolean, optional, default ``no`` (legacy behavior). * ``username`` - GitHub authorization user, string, required. Basically the same as ``owner``. @@ -411,6 +417,8 @@ This feature requires GitHub key creation (see below). Section name must be eith Section name must be either ``remote-service`` (plus optional architecture name, e.g. ``remote-service:x86_64``) or random name with ``type`` set. * ``type`` - type of the report, string, optional, must be set to ``remote-service`` if exists. +* ``max_retries`` - maximum amount of retries of HTTP requests, integer, optional, default ``0``. +* ``retry_backoff`` - retry exponential backoff, float, optional, default ``0.0``. * ``timeout`` - HTTP request timeout in seconds, integer, optional, default is ``30``. ``rsync`` type diff --git a/package/share/ahriman/settings/ahriman.ini b/package/share/ahriman/settings/ahriman.ini index d983b3dd..0350485a 100644 --- a/package/share/ahriman/settings/ahriman.ini +++ b/package/share/ahriman/settings/ahriman.ini @@ -73,8 +73,12 @@ enabled = yes ; In case if unix sockets are used, it might point to the valid socket with encoded path, e.g.: ; address = http+unix://%2Fvar%2Flib%2Fahriman%2Fsocket ;address = http://${web:host}:${web:port} +; Maximum amount of retries of HTTP requests. +;max_retries = 0 ; Optional password for authentication (if enabled). ;password = +; Retry exponential backoff. +;retry_backoff = 0.0 ; Do not log HTTP errors if occurs. suppress_http_log_errors = yes ; HTTP request timeout in seconds. @@ -216,6 +220,10 @@ templates[] = ${prefix}/share/ahriman/templates ;homepage= ; Prefix for packages links. Link to a package will be formed as link_path / filename. ;link_path = +; Maximum amount of retries of HTTP requests. +;max_retries = 0 +; Retry exponential backoff. +;retry_backoff = 0.0 ; Optional link to the RSS feed. ;rss_url = ; Template name to be used. @@ -236,12 +244,16 @@ target = [github] ; Trigger type name. ;type = github +; Maximum amount of retries of HTTP requests. +;max_retries = 0 ; GitHub repository owner username. ;owner = ; GitHub API key. public_repo (repo) scope is required. ;password = ; GitHub repository name. ;repository = +; Retry exponential backoff. +;retry_backoff = 0.0 ; HTTP request timeout in seconds. ;timeout = 30 ; Include repository name to release name (recommended). @@ -253,6 +265,10 @@ target = [remote-service] ; Trigger type name. ;type = remote-service +; Maximum amount of retries of HTTP requests. +;max_retries = 0 +; Retry exponential backoff. +;retry_backoff = 0.0 ; HTTP request timeout in seconds. ;timeout = 30 diff --git a/src/ahriman/core/configuration/schema.py b/src/ahriman/core/configuration/schema.py index 9d7746f9..6d340c40 100644 --- a/src/ahriman/core/configuration/schema.py +++ b/src/ahriman/core/configuration/schema.py @@ -296,10 +296,20 @@ CONFIGURATION_SCHEMA: ConfigurationSchema = { "empty": False, "is_url": [], }, + "max_retries": { + "type": "integer", + "coerce": "integer", + "min": 0, + }, "password": { "type": "string", "empty": False, }, + "retry_backoff": { + "type": "float", + "coerce": "float", + "min": 0, + }, "suppress_http_log_errors": { "type": "boolean", "coerce": "boolean", diff --git a/src/ahriman/core/configuration/validator.py b/src/ahriman/core/configuration/validator.py index 20233d79..aea43554 100644 --- a/src/ahriman/core/configuration/validator.py +++ b/src/ahriman/core/configuration/validator.py @@ -76,6 +76,19 @@ class Validator(RootValidator): converted: bool = self.configuration._convert_to_boolean(value) # type: ignore[attr-defined] return converted + def _normalize_coerce_float(self, value: str) -> float: + """ + extract float from string value + + Args: + value(str): converting value + + Returns: + float: value converted to float according to configuration rules + """ + del self + return float(value) + def _normalize_coerce_integer(self, value: str) -> int: """ extract integer from string value diff --git a/src/ahriman/core/http/sync_ahriman_client.py b/src/ahriman/core/http/sync_ahriman_client.py index 17abdabe..a69c1881 100644 --- a/src/ahriman/core/http/sync_ahriman_client.py +++ b/src/ahriman/core/http/sync_ahriman_client.py @@ -20,10 +20,9 @@ import contextlib import requests -from functools import cached_property +from requests.adapters import BaseAdapter from urllib.parse import urlparse -from ahriman import __version__ from ahriman.core.http.sync_http_client import SyncHttpClient @@ -37,32 +36,36 @@ class SyncAhrimanClient(SyncHttpClient): address: str - @cached_property - def session(self) -> requests.Session: + def _login_url(self) -> str: """ - get or create session + get url for the login api Returns: - request.Session: created session object + str: full url for web service to log in """ - if urlparse(self.address).scheme == "http+unix": - import requests_unixsocket - session: requests.Session = requests_unixsocket.Session() # type: ignore[no-untyped-call] - session.headers["User-Agent"] = f"ahriman/{__version__}" - return session + return f"{self.address}/api/v1/login" - session = requests.Session() - session.headers["User-Agent"] = f"ahriman/{__version__}" - self._login(session) - - return session - - def _login(self, session: requests.Session) -> None: + def adapters(self) -> dict[str, BaseAdapter]: """ - process login to the service + get registered adapters + + Returns: + dict[str, BaseAdapter]: map of protocol and adapter used for this protocol + """ + adapters = SyncHttpClient.adapters(self) + + if (scheme := urlparse(self.address).scheme) == "http+unix": + from requests_unixsocket.adapters import UnixAdapter + adapters[f"{scheme}://"] = UnixAdapter() # type: ignore[no-untyped-call] + + return adapters + + def start(self, session: requests.Session) -> None: + """ + method which will be called on session creation Args: - session(requests.Session): request session to login + session(requests.Session): created requests session """ if self.auth is None: return # no auth configured @@ -74,12 +77,3 @@ class SyncAhrimanClient(SyncHttpClient): } with contextlib.suppress(Exception): self.make_request("POST", self._login_url(), json=payload, session=session) - - def _login_url(self) -> str: - """ - get url for the login api - - Returns: - str: full url for web service to log in - """ - return f"{self.address}/api/v1/login" diff --git a/src/ahriman/core/http/sync_http_client.py b/src/ahriman/core/http/sync_http_client.py index 75cdc2be..3ee5ea6e 100644 --- a/src/ahriman/core/http/sync_http_client.py +++ b/src/ahriman/core/http/sync_http_client.py @@ -21,7 +21,9 @@ import requests import sys from functools import cached_property +from requests.adapters import BaseAdapter, HTTPAdapter from typing import Any, IO, Literal +from urllib3.util.retry import Retry from ahriman import __version__ from ahriman.core.configuration import Configuration @@ -62,6 +64,16 @@ class SyncHttpClient(LazyLogging): self.timeout: int | None = configuration.getint(section, "timeout", fallback=30) self.suppress_errors = suppress_errors + retries = configuration.getint(section, "max_retries", fallback=0) + self.retry = Retry( + total=retries, + connect=retries, + read=retries, + status=retries, + status_forcelist=[429, 500, 502, 503, 504], + backoff_factor=configuration.getfloat(section, "retry_backoff", fallback=0.0), + ) + @cached_property def session(self) -> requests.Session: """ @@ -71,11 +83,17 @@ class SyncHttpClient(LazyLogging): request.Session: created session object """ session = requests.Session() + + for protocol, adapter in self.adapters().items(): + session.mount(protocol, adapter) + python_version = ".".join(map(str, sys.version_info[:3])) # just major.minor.patch session.headers["User-Agent"] = f"ahriman/{__version__} " \ f"{requests.utils.default_user_agent()} " \ f"python/{python_version}" + self.start(session) + return session @staticmethod @@ -92,6 +110,19 @@ class SyncHttpClient(LazyLogging): result: str = exception.response.text if exception.response is not None else "" return result + def adapters(self) -> dict[str, BaseAdapter]: + """ + get registered adapters + + Returns: + dict[str, BaseAdapter]: map of protocol and adapter used for this protocol + """ + adapter = HTTPAdapter(max_retries=self.retry) + return { + "http://": adapter, + "https://": adapter, + } + def make_request(self, method: Literal["DELETE", "GET", "HEAD", "POST", "PUT"], url: str, *, headers: dict[str, str] | None = None, params: list[tuple[str, str]] | None = None, @@ -139,3 +170,11 @@ class SyncHttpClient(LazyLogging): if not suppress_errors: self.logger.exception("could not perform http request") raise + + def start(self, session: requests.Session) -> None: + """ + method which will be called on session creation + + Args: + session(requests.Session): created requests session + """ diff --git a/src/ahriman/core/report/report_trigger.py b/src/ahriman/core/report/report_trigger.py index 7347e0e9..75940a53 100644 --- a/src/ahriman/core/report/report_trigger.py +++ b/src/ahriman/core/report/report_trigger.py @@ -302,6 +302,16 @@ class ReportTrigger(Trigger): "empty": False, "is_url": [], }, + "max_retries": { + "type": "integer", + "coerce": "integer", + "min": 0, + }, + "retry_backoff": { + "type": "float", + "coerce": "float", + "min": 0, + }, "rss_url": { "type": "string", "empty": False, diff --git a/src/ahriman/core/upload/upload_trigger.py b/src/ahriman/core/upload/upload_trigger.py index 1ff1bc54..c5c065e0 100644 --- a/src/ahriman/core/upload/upload_trigger.py +++ b/src/ahriman/core/upload/upload_trigger.py @@ -54,6 +54,11 @@ class UploadTrigger(Trigger): "type": "string", "allowed": ["github"], }, + "max_retries": { + "type": "integer", + "coerce": "integer", + "min": 0, + }, "owner": { "type": "string", "required": True, @@ -68,6 +73,11 @@ class UploadTrigger(Trigger): "required": True, "empty": False, }, + "retry_backoff": { + "type": "float", + "coerce": "float", + "min": 0, + }, "timeout": { "type": "integer", "coerce": "integer", @@ -90,6 +100,16 @@ class UploadTrigger(Trigger): "type": "string", "allowed": ["ahriman", "remote-service"], }, + "max_retries": { + "type": "integer", + "coerce": "integer", + "min": 0, + }, + "retry_backoff": { + "type": "float", + "coerce": "float", + "min": 0, + }, "timeout": { "type": "integer", "coerce": "integer", diff --git a/tests/ahriman/core/configuration/test_validator.py b/tests/ahriman/core/configuration/test_validator.py index 1abc4b75..039e4bb6 100644 --- a/tests/ahriman/core/configuration/test_validator.py +++ b/tests/ahriman/core/configuration/test_validator.py @@ -33,6 +33,14 @@ def test_normalize_coerce_boolean(validator: Validator, mocker: MockerFixture) - convert_mock.assert_called_once_with("1") +def test_normalize_coerce_float(validator: Validator) -> None: + """ + must convert string value to float by using configuration converters + """ + assert validator._normalize_coerce_float("1.5") == 1.5 + assert validator._normalize_coerce_float("0.0") == 0.0 + + def test_normalize_coerce_integer(validator: Validator) -> None: """ must convert string value to integer by using configuration converters diff --git a/tests/ahriman/core/http/test_sync_ahriman_client.py b/tests/ahriman/core/http/test_sync_ahriman_client.py index 4d7e42ea..c0189904 100644 --- a/tests/ahriman/core/http/test_sync_ahriman_client.py +++ b/tests/ahriman/core/http/test_sync_ahriman_client.py @@ -1,6 +1,5 @@ import pytest import requests -import requests_unixsocket from pytest_mock import MockerFixture @@ -8,31 +7,32 @@ from ahriman.core.http import SyncAhrimanClient from ahriman.models.user import User -def test_session(ahriman_client: SyncAhrimanClient, mocker: MockerFixture) -> None: +def test_adapters(ahriman_client: SyncAhrimanClient) -> None: """ - must create normal requests session + must return native adapters """ - login_mock = mocker.patch("ahriman.core.http.SyncAhrimanClient._login") - - assert isinstance(ahriman_client.session, requests.Session) - assert not isinstance(ahriman_client.session, requests_unixsocket.Session) - login_mock.assert_called_once_with(pytest.helpers.anyvar(int)) + assert "http+unix://" not in ahriman_client.adapters() -def test_session_unix_socket(ahriman_client: SyncAhrimanClient, mocker: MockerFixture) -> None: +def test_adapters_unix_socket(ahriman_client: SyncAhrimanClient) -> None: """ - must create unix socket session + must register unix socket adapter """ - login_mock = mocker.patch("ahriman.core.http.SyncAhrimanClient._login") ahriman_client.address = "http+unix://path" - - assert isinstance(ahriman_client.session, requests_unixsocket.Session) - login_mock.assert_not_called() + assert "http+unix://" in ahriman_client.adapters() -def test_login(ahriman_client: SyncAhrimanClient, user: User, mocker: MockerFixture) -> None: +def test_login_url(ahriman_client: SyncAhrimanClient) -> None: """ - must login user + must generate login url correctly + """ + assert ahriman_client._login_url().startswith(ahriman_client.address) + assert ahriman_client._login_url().endswith("/api/v1/login") + + +def test_start(ahriman_client: SyncAhrimanClient, user: User, mocker: MockerFixture) -> None: + """ + must log in user on start """ ahriman_client.auth = (user.username, user.password) requests_mock = mocker.patch("ahriman.core.http.SyncAhrimanClient.make_request") @@ -42,40 +42,32 @@ def test_login(ahriman_client: SyncAhrimanClient, user: User, mocker: MockerFixt } session = requests.Session() - ahriman_client._login(session) + ahriman_client.start(session) requests_mock.assert_called_once_with("POST", pytest.helpers.anyvar(str, True), json=payload, session=session) -def test_login_failed(ahriman_client: SyncAhrimanClient, user: User, mocker: MockerFixture) -> None: +def test_start_failed(ahriman_client: SyncAhrimanClient, user: User, mocker: MockerFixture) -> None: """ - must suppress any exception happened during login + must suppress any exception happened during session start """ ahriman_client.user = user mocker.patch("requests.Session.request", side_effect=Exception) - ahriman_client._login(requests.Session()) + ahriman_client.start(requests.Session()) -def test_login_failed_http_error(ahriman_client: SyncAhrimanClient, user: User, mocker: MockerFixture) -> None: +def test_start_failed_http_error(ahriman_client: SyncAhrimanClient, user: User, mocker: MockerFixture) -> None: """ - must suppress HTTP exception happened during login + must suppress HTTP exception happened during session start """ ahriman_client.user = user mocker.patch("requests.Session.request", side_effect=requests.HTTPError) - ahriman_client._login(requests.Session()) + ahriman_client.start(requests.Session()) -def test_login_skip(ahriman_client: SyncAhrimanClient, mocker: MockerFixture) -> None: +def test_start_skip(ahriman_client: SyncAhrimanClient, mocker: MockerFixture) -> None: """ must skip login if no user set """ requests_mock = mocker.patch("requests.Session.request") - ahriman_client._login(requests.Session()) + ahriman_client.start(requests.Session()) requests_mock.assert_not_called() - - -def test_login_url(ahriman_client: SyncAhrimanClient) -> None: - """ - must generate login url correctly - """ - assert ahriman_client._login_url().startswith(ahriman_client.address) - assert ahriman_client._login_url().endswith("/api/v1/login") diff --git a/tests/ahriman/core/http/test_sync_http_client.py b/tests/ahriman/core/http/test_sync_http_client.py index c7a194fd..38588fb5 100644 --- a/tests/ahriman/core/http/test_sync_http_client.py +++ b/tests/ahriman/core/http/test_sync_http_client.py @@ -33,12 +33,15 @@ def test_init_auth_empty() -> None: assert SyncHttpClient().auth is None -def test_session() -> None: +def test_session(mocker: MockerFixture) -> None: """ must generate valid session """ + start_mock = mocker.patch("ahriman.core.http.sync_http_client.SyncHttpClient.start") + session = SyncHttpClient().session assert "User-Agent" in session.headers + start_mock.assert_called_once_with(pytest.helpers.anyvar(int)) def test_exception_response_text() -> None: @@ -60,6 +63,18 @@ def test_exception_response_text_empty() -> None: assert SyncHttpClient.exception_response_text(exception) == "" +def test_adapters() -> None: + """ + must create adapters with retry policy + """ + client = SyncHttpClient() + adapers = client.adapters() + + assert "http://" in adapers + assert "https://" in adapers + assert all(adapter.max_retries == client.retry for adapter in adapers.values()) + + def test_make_request(mocker: MockerFixture) -> None: """ must make HTTP request @@ -158,3 +173,11 @@ def test_make_request_session() -> None: session_mock.request.assert_called_once_with( "GET", "url", params=None, data=None, headers=None, files=None, json=None, stream=None, auth=None, timeout=client.timeout) + + +def test_start() -> None: + """ + must do nothing on start + """ + client = SyncHttpClient() + client.start(client.session)