add retry on authorization errros

.github/workflows/setup.sh

@@ -10,7 +10,7 @@ echo -e '[arcanisrepo]\nServer = http://repo.arcanis.me/$arch\nSigLevel = Never'
 # refresh the image
 pacman --noconfirm -Syu
 # main dependencies
-pacman --noconfirm -Sy base-devel devtools git pyalpm python-cerberus python-inflection python-passlib python-requests python-srcinfo python-systemd sudo
+pacman --noconfirm -Sy base-devel devtools git pyalpm python-cerberus python-inflection python-passlib python-requests python-srcinfo python-tenacity sudo
 # make dependencies
 pacman --noconfirm -Sy python-build python-flit python-installer python-wheel
 # optional dependencies

Dockerfile

@@ -31,7 +31,7 @@ RUN useradd -m -d "/home/build" -s "/usr/bin/nologin" build && \
 COPY "docker/install-aur-package.sh" "/usr/local/bin/install-aur-package"
 ## install package dependencies
 ## darcs is not installed by reasons, because it requires a lot haskell packages which dramatically increase image size
-RUN pacman -Sy --noconfirm --asdeps devtools git pyalpm python-cerberus python-inflection python-passlib python-requests python-srcinfo && \
+RUN pacman -Sy --noconfirm --asdeps devtools git pyalpm python-cerberus python-inflection python-passlib python-requests python-srcinfo python-tenacity && \
     pacman -Sy --noconfirm --asdeps python-build python-flit python-installer python-wheel && \
     pacman -Sy --noconfirm --asdeps breezy mercurial python-aiohttp python-aiohttp-cors python-boto3 python-cryptography python-jinja python-requests-unixsocket python-systemd rsync subversion && \
     runuser -u build -- install-aur-package python-aioauth-client python-aiohttp-apispec-git python-aiohttp-jinja2  \

docs/ahriman.core.http.rst

@@ -4,6 +4,14 @@ ahriman.core.http package
 Submodules
 ----------
 
+ahriman.core.http.sync\_ahriman\_client module
+----------------------------------------------
+
+.. automodule:: ahriman.core.http.sync_ahriman_client
+   :members:
+   :no-undoc-members:
+   :show-inheritance:
+
 ahriman.core.http.sync\_http\_client module
 -------------------------------------------
 

docs/configuration.rst

@@ -111,6 +111,7 @@ Reporting to web service related settings. In most cases there is fallback to we
 * ``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.
 * ``password`` - password to authorize in web service in order to update service status, string, required in case if authorization enabled.
 * ``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.
 
 ``web`` group
@@ -129,7 +130,6 @@ Web server settings. If any of ``host``/``port`` is not set, web integration wil
 * ``port`` - port to bind, integer, optional.
 * ``static_path`` - path to directory with static files, string, required.
 * ``templates`` - path to templates directories, space separated list of strings, required.
-* ``timeout`` - HTTP request timeout in seconds, integer, optional, default is ``30``.
 * ``unix_socket`` - path to the listening unix socket, string, optional. If set, server will create the socket on the specified address which can (and will) be used by application. Note, that unlike usual host/port configuration, unix socket allows to perform requests without authorization.
 * ``unix_socket_unsafe`` - set unsafe (o+w) permissions to unix socket, boolean, optional, default ``yes``. This option is enabled by default, because it is supposed that unix socket is created in safe environment (only web service is supposed to be used in unsafe), but it can be disabled by configuration.
 * ``wait_timeout`` - wait timeout in seconds, maximum amount of time to be waited before lock will be free, integer, optional.

package/archlinux/PKGBUILD

@@ -7,7 +7,7 @@ pkgdesc="ArcH linux ReposItory MANager"
 arch=('any')
 url="https://github.com/arcan1s/ahriman"
 license=('GPL3')
-depends=('devtools>=1:1.0.0' 'git' 'pyalpm' 'python-cerberus' 'python-inflection' 'python-passlib' 'python-requests' 'python-srcinfo')
+depends=('devtools>=1:1.0.0' 'git' 'pyalpm' 'python-cerberus' 'python-inflection' 'python-passlib' 'python-requests' 'python-srcinfo' 'python-tenacity')
 makedepends=('python-build' 'python-flit' 'python-installer' 'python-wheel')
 optdepends=('breezy: -bzr packages support'
             'darcs: -darcs packages support'

pyproject.toml

@@ -21,6 +21,7 @@ dependencies = [
     "passlib",
     "requests",
     "srcinfo",
+    "tenacity",
 ]
 
 dynamic = ["version"]

src/ahriman/core/configuration/schema.py

@@ -269,6 +269,11 @@ CONFIGURATION_SCHEMA: ConfigurationSchema = {
                 "type": "boolean",
                 "coerce": "boolean",
             },
+            "timeout": {
+                "type": "integer",
+                "coerce": "integer",
+                "min": 0,
+            },
             "username": {
                 "type": "string",
                 "empty": False,

src/ahriman/core/http/__init__.py

@@ -17,4 +17,5 @@
 # You should have received a copy of the GNU General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 #
+from ahriman.core.http.sync_ahriman_client import SyncAhrimanClient
 from ahriman.core.http.sync_http_client import MultipartType, SyncHttpClient

src/ahriman/core/http/sync_ahriman_client.py

@@ -0,0 +1,134 @@
+#
+# Copyright (c) 2021-2023 ahriman team.
+#
+# This file is part of ahriman
+# (see https://github.com/arcan1s/ahriman).
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+import contextlib
+import requests
+import tenacity
+
+from functools import cached_property
+from typing import Any
+from urllib.parse import urlparse
+
+from ahriman import __version__
+from ahriman.core.http.sync_http_client import SyncHttpClient
+
+
+class SyncAhrimanClient(SyncHttpClient):
+    """
+    wrapper for ahriman web service
+
+    Attributes:
+        address(str): address of the web service
+    """
+
+    address: str
+
+    @cached_property
+    def session(self) -> requests.Session:
+        """
+        get or create session
+
+        Returns:
+            request.Session: created session object
+        """
+        if urlparse(self.address).scheme == "http+unix":
+            import requests_unixsocket  # type: ignore[import-untyped]
+            session: requests.Session = requests_unixsocket.Session()
+            session.headers["User-Agent"] = f"ahriman/{__version__}"
+            return session
+
+        session = requests.Session()
+        session.headers["User-Agent"] = f"ahriman/{__version__}"
+        self._login(session)
+
+        return session
+
+    @staticmethod
+    def is_retry_allowed(exception: BaseException) -> bool:
+        """
+        check if retry is allowed for the exception
+
+        Args:
+            exception(BaseException): exception raised
+
+        Returns:
+            bool: True in case if exception is in white list and false otherwise
+        """
+        if not isinstance(exception, requests.RequestException):
+            return False  # not a request exception
+        status_code = exception.response.status_code if exception.response is not None else None
+        return status_code in (401,)
+
+    @staticmethod
+    def on_retry(state: tenacity.RetryCallState) -> None:
+        """
+        action to be called before retry
+
+        Args:
+            state(tenacity.RetryCallState): current retry call state
+        """
+        instance = next(arg for arg in state.args if isinstance(arg, SyncAhrimanClient))
+        if hasattr(instance, "session"):  # only if it was initialized
+            del instance.session  # clear session
+
+    def _login(self, session: requests.Session) -> None:
+        """
+        process login to the service
+
+        Args:
+            session(requests.Session): request session to login
+        """
+        if self.auth is None:
+            return  # no auth configured
+
+        username, password = self.auth
+        payload = {
+            "username": username,
+            "password": password,
+        }
+        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"
+
+    @tenacity.retry(
+        stop=tenacity.stop_after_attempt(2),
+        retry=tenacity.retry_if_exception(is_retry_allowed),
+        after=on_retry,
+        reraise=True,
+    )
+    def make_request(self, *args: Any, **kwargs: Any) -> requests.Response:
+        """
+        perform request with specified parameters
+
+        Args:
+            *args(Any): request method positional arguments
+            **kwargs(Any): request method keyword arguments
+
+        Returns:
+            requests.Response: response object
+        """
+        return SyncHttpClient.make_request(self, *args, **kwargs)

src/ahriman/core/http/sync_http_client.py

@@ -77,12 +77,12 @@ class SyncHttpClient(LazyLogging):
         return session
 
     @staticmethod
-    def exception_response_text(exception: requests.exceptions.RequestException) -> str:
+    def exception_response_text(exception: requests.RequestException) -> str:
         """
         safe response exception text generation
 
         Args:
-            exception(requests.exceptions.RequestException): exception raised
+            exception(requests.RequestException): exception raised
 
         Returns:
             str: text of the response if it is not None and empty string otherwise

src/ahriman/core/status/web_client.py

@@ -19,14 +19,11 @@
 #
 import contextlib
 import logging
-import requests
 
-from functools import cached_property
-from urllib.parse import quote_plus as urlencode, urlparse
+from urllib.parse import quote_plus as urlencode
 
-from ahriman import __version__
 from ahriman.core.configuration import Configuration
-from ahriman.core.http import SyncHttpClient
+from ahriman.core.http import SyncAhrimanClient
 from ahriman.core.status.client import Client
 from ahriman.models.build_status import BuildStatus, BuildStatusEnum
 from ahriman.models.internal_status import InternalStatus
@@ -35,12 +32,11 @@ from ahriman.models.package import Package
 from ahriman.models.repository_id import RepositoryId
 
 
-class WebClient(Client, SyncHttpClient):
+class WebClient(Client, SyncAhrimanClient):
     """
     build status reporter web client
 
     Attributes:
-        address(str): address of the web service
         repository_id(RepositoryId): repository unique identifier
     """
 
@@ -56,30 +52,10 @@ class WebClient(Client, SyncHttpClient):
         suppress_errors = configuration.getboolean(  # read old-style first and then fallback to new style
             "settings", "suppress_http_log_errors",
             fallback=configuration.getboolean("status", "suppress_http_log_errors", fallback=False))
-        SyncHttpClient.__init__(self, configuration, section, suppress_errors=suppress_errors)
+        SyncAhrimanClient.__init__(self, configuration, section, suppress_errors=suppress_errors)
 
         self.repository_id = repository_id
 
-    @cached_property
-    def session(self) -> requests.Session:
-        """
-        get or create session
-
-        Returns:
-            request.Session: created session object
-        """
-        if urlparse(self.address).scheme == "http+unix":
-            import requests_unixsocket  # type: ignore[import-untyped]
-            session: requests.Session = requests_unixsocket.Session()
-            session.headers["User-Agent"] = f"ahriman/{__version__}"
-            return session
-
-        session = requests.Session()
-        session.headers["User-Agent"] = f"ahriman/{__version__}"
-        self._login(session)
-
-        return session
-
     @staticmethod
     def parse_address(configuration: Configuration) -> tuple[str, str]:
         """
@@ -107,33 +83,6 @@ class WebClient(Client, SyncHttpClient):
             address = f"http://{host}:{port}"
         return "web", address
 
-    def _login(self, session: requests.Session) -> None:
-        """
-        process login to the service
-
-        Args:
-            session(requests.Session): request session to login
-        """
-        if self.auth is None:
-            return  # no auth configured
-
-        username, password = self.auth
-        payload = {
-            "username": username,
-            "password": password,
-        }
-        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"
-
     def _logs_url(self, package_base: str) -> str:
         """
         get url for the logs api

tests/ahriman/core/http/conftest.py

@@ -0,0 +1,21 @@
+import pytest
+
+from ahriman.core.configuration import Configuration
+from ahriman.core.http import SyncAhrimanClient
+from ahriman.core.status.web_client import WebClient
+
+
+@pytest.fixture
+def ahriman_client(configuration: Configuration) -> SyncAhrimanClient:
+    """
+    ahriman web client fixture
+
+    Args:
+        configuration(Configuration): configuration fixture
+
+    Returns:
+        SyncAhrimanClient: ahriman web client test instance
+    """
+    configuration.set("web", "port", "8080")
+    _, repository_id = configuration.check_loaded()
+    return WebClient(repository_id, configuration)

tests/ahriman/core/http/test_sync_ahriman_client.py

@@ -0,0 +1,146 @@
+import pytest
+import requests
+import requests_unixsocket
+import tenacity
+
+from pytest_mock import MockerFixture
+from unittest.mock import call as MockCall
+
+from ahriman.core.http import SyncAhrimanClient
+from ahriman.models.user import User
+
+
+def test_session(ahriman_client: SyncAhrimanClient, mocker: MockerFixture) -> None:
+    """
+    must create normal requests session
+    """
+    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))
+
+
+def test_session_unix_socket(ahriman_client: SyncAhrimanClient, mocker: MockerFixture) -> None:
+    """
+    must create unix socket session
+    """
+    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()
+
+
+def test_is_retry_allowed() -> None:
+    """
+    must allow retries on 401 errors
+    """
+    assert not SyncAhrimanClient.is_retry_allowed(Exception())
+
+    response = requests.Response()
+    response.status_code = 400
+    assert not SyncAhrimanClient.is_retry_allowed(requests.HTTPError(response=response))
+
+    response.status_code = 401
+    assert SyncAhrimanClient.is_retry_allowed(requests.HTTPError(response=response))
+
+    response.status_code = 403
+    assert not SyncAhrimanClient.is_retry_allowed(requests.HTTPError(response=response))
+
+
+def test_on_retry(ahriman_client: SyncAhrimanClient) -> None:
+    """
+    must remove session on retry
+    """
+    SyncAhrimanClient.on_retry(
+        tenacity.RetryCallState(
+            retry_object=tenacity.Retrying(),
+            fn=SyncAhrimanClient.make_request,
+            args=(ahriman_client,),
+            kwargs={},
+        )
+    )
+
+
+def test_login(ahriman_client: SyncAhrimanClient, user: User, mocker: MockerFixture) -> None:
+    """
+    must login user
+    """
+    ahriman_client.auth = (user.username, user.password)
+    requests_mock = mocker.patch("ahriman.core.http.SyncAhrimanClient.make_request")
+    payload = {
+        "username": user.username,
+        "password": user.password
+    }
+    session = requests.Session()
+
+    ahriman_client._login(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:
+    """
+    must suppress any exception happened during login
+    """
+    ahriman_client.user = user
+    mocker.patch("requests.Session.request", side_effect=Exception())
+    ahriman_client._login(requests.Session())
+
+
+def test_login_failed_http_error(ahriman_client: SyncAhrimanClient, user: User, mocker: MockerFixture) -> None:
+    """
+    must suppress HTTP exception happened during login
+    """
+    ahriman_client.user = user
+    mocker.patch("requests.Session.request", side_effect=requests.HTTPError())
+    ahriman_client._login(requests.Session())
+
+
+def test_login_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())
+    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")
+
+
+def test_make_request_retry(ahriman_client: SyncAhrimanClient, mocker: MockerFixture) -> None:
+    """
+    must retry HTTP request
+    """
+    response_ok = requests.Response()
+    response_ok.status_code = 200
+    response_error = requests.Response()
+    response_error.status_code = 401
+    # login on init -> request with error -> login on error -> request without error
+    request_mock = mocker.patch("requests.Session.request", side_effect=[
+        response_ok,
+        response_error,
+        response_ok,
+        response_ok,
+    ])
+
+    ahriman_client.auth = ("username", "password")
+    assert ahriman_client.make_request("GET", "url") is not None
+    request_mock.assert_has_calls([
+        MockCall("POST", "http://127.0.0.1:8080/api/v1/login", params=None, data=None, headers=None, files=None,
+                 json={"username": "username", "password": "password"},
+                 auth=ahriman_client.auth, timeout=ahriman_client.timeout),
+        MockCall("GET", "url", params=None, data=None, headers=None, files=None, json=None,
+                 auth=ahriman_client.auth, timeout=ahriman_client.timeout),
+        MockCall("POST", "http://127.0.0.1:8080/api/v1/login", params=None, data=None, headers=None, files=None,
+                 json={"username": "username", "password": "password"},
+                 auth=ahriman_client.auth, timeout=ahriman_client.timeout),
+        MockCall("GET", "url", params=None, data=None, headers=None, files=None, json=None,
+                 auth=ahriman_client.auth, timeout=ahriman_client.timeout),
+    ])

tests/ahriman/core/status/test_web_client.py

@@ -2,7 +2,6 @@ import json
 import logging
 import pytest
 import requests
-import requests_unixsocket
 
 from pytest_mock import MockerFixture
 
@@ -12,29 +11,6 @@ from ahriman.models.build_status import BuildStatus, BuildStatusEnum
 from ahriman.models.internal_status import InternalStatus
 from ahriman.models.log_record_id import LogRecordId
 from ahriman.models.package import Package
-from ahriman.models.user import User
-
-
-def test_session(web_client: WebClient, mocker: MockerFixture) -> None:
-    """
-    must create normal requests session
-    """
-    login_mock = mocker.patch("ahriman.core.status.web_client.WebClient._login")
-
-    assert isinstance(web_client.session, requests.Session)
-    assert not isinstance(web_client.session, requests_unixsocket.Session)
-    login_mock.assert_called_once_with(pytest.helpers.anyvar(int))
-
-
-def test_session_unix_socket(web_client: WebClient, mocker: MockerFixture) -> None:
-    """
-    must create unix socket session
-    """
-    login_mock = mocker.patch("ahriman.core.status.web_client.WebClient._login")
-    web_client.address = "http+unix://path"
-
-    assert isinstance(web_client.session, requests_unixsocket.Session)
-    login_mock.assert_not_called()
 
 
 def test_parse_address(configuration: Configuration) -> None:
@@ -55,57 +31,6 @@ def test_parse_address(configuration: Configuration) -> None:
     assert WebClient.parse_address(configuration) == ("status", "http://localhost:8082")
 
 
-def test_login(web_client: WebClient, user: User, mocker: MockerFixture) -> None:
-    """
-    must login user
-    """
-    web_client.auth = (user.username, user.password)
-    requests_mock = mocker.patch("ahriman.core.status.web_client.WebClient.make_request")
-    payload = {
-        "username": user.username,
-        "password": user.password
-    }
-    session = requests.Session()
-
-    web_client._login(session)
-    requests_mock.assert_called_once_with("POST", pytest.helpers.anyvar(str, True), json=payload, session=session)
-
-
-def test_login_failed(web_client: WebClient, user: User, mocker: MockerFixture) -> None:
-    """
-    must suppress any exception happened during login
-    """
-    web_client.user = user
-    mocker.patch("requests.Session.request", side_effect=Exception())
-    web_client._login(requests.Session())
-
-
-def test_login_failed_http_error(web_client: WebClient, user: User, mocker: MockerFixture) -> None:
-    """
-    must suppress HTTP exception happened during login
-    """
-    web_client.user = user
-    mocker.patch("requests.Session.request", side_effect=requests.HTTPError())
-    web_client._login(requests.Session())
-
-
-def test_login_skip(web_client: WebClient, mocker: MockerFixture) -> None:
-    """
-    must skip login if no user set
-    """
-    requests_mock = mocker.patch("requests.Session.request")
-    web_client._login(requests.Session())
-    requests_mock.assert_not_called()
-
-
-def test_login_url(web_client: WebClient) -> None:
-    """
-    must generate login url correctly
-    """
-    assert web_client._login_url().startswith(web_client.address)
-    assert web_client._login_url().endswith("/api/v1/login")
-
-
 def test_status_url(web_client: WebClient) -> None:
     """
     must generate package status url correctly
@@ -377,11 +302,13 @@ def test_status_update(web_client: WebClient, mocker: MockerFixture) -> None:
     requests_mock = mocker.patch("ahriman.core.status.web_client.WebClient.make_request")
 
     web_client.status_update(BuildStatusEnum.Unknown)
-    requests_mock.assert_called_once_with("POST", pytest.helpers.anyvar(str, True),
-                                          params=web_client.repository_id.query(),
-                                          json={
-                                              "status": BuildStatusEnum.Unknown.value,
-    })
+    requests_mock.assert_called_once_with(
+        "POST", pytest.helpers.anyvar(str, True),
+        params=web_client.repository_id.query(),
+        json={
+            "status": BuildStatusEnum.Unknown.value,
+        }
+    )
 
 
 def test_status_update_self_failed(web_client: WebClient, mocker: MockerFixture) -> None: