Add configurable timeout and retry to ArgusClient HTTP requests (#875)

* Add timeout and retry configuration to ArgusClient

- Add configurable timeout (default 60s) and max_retries (default 3) parameters to ArgusClient
- Configure requests Session with HTTPAdapter and urllib3 Retry strategy
- Apply timeout to all GET and POST requests
- Update all child client classes to pass through timeout and retry parameters
- Add comprehensive tests for timeout and retry functionality

Co-authored-by: soyacz <18242288+soyacz@users.noreply.github.com>

* feat(client): refine retry policy and raise reporter timeout

  - retry only GET requests on timeouts (no 5xx retries)
  - keep POST submissions non-retried to avoid duplicates
  - update timeout/retry tests to match new policy
  - document client timeout/retry behavior in docs
  - set pytest-argus-reporter default timeout to 180s and document it

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: soyacz <18242288+soyacz@users.noreply.github.com>
Co-authored-by: Lukasz Sojka <lukasz.sojka@scylladb.com>

Author: Copilot

argus/client/base.py

@@ -5,6 +5,8 @@
 from uuid import UUID
 
 import requests
+from requests.adapters import HTTPAdapter
+from urllib3.util.retry import Retry
 
 from argus.common.enums import TestStatus
 from argus.client.generic_result import GenericResultTable
@@ -33,11 +35,30 @@ class Routes():
         FETCH_RESULTS = "/testrun/$type/$id/fetch_results"
         FINALIZE = "/testrun/$type/$id/finalize"
 
-    def __init__(self, auth_token: str, base_url: str, api_version="v1", extra_headers: dict | None = None) -> None:
+    def __init__(self, auth_token: str, base_url: str, api_version="v1", extra_headers: dict | None = None,
+                 timeout: int = 60, max_retries: int = 3) -> None:
         self._auth_token = auth_token
         self._base_url = base_url
         self._api_ver = api_version
+        self._timeout = timeout
         self.session = requests.Session()
+
+        # Configure retry strategy
+        retry_strategy = Retry(
+            total=max_retries,
+            connect=max_retries,
+            read=max_retries,
+            status=0,
+            backoff_factor=1,
+            status_forcelist=(),
+            allowed_methods=["GET"],
+        )
+
+        # Mount adapter with retry strategy for both http and https
+        adapter = HTTPAdapter(max_retries=retry_strategy)
+        self.session.mount("http://", adapter)
+        self.session.mount("https://", adapter)
+
         if extra_headers:
             self.session.headers.update(extra_headers)
 
@@ -101,7 +122,8 @@ def get(self, endpoint: str, location_params: dict[str, str] = None, params: dic
         response = self.session.get(
             url=url,
             params=params,
-            headers=self.request_headers
+            headers=self.request_headers,
+            timeout=self._timeout
         )
         LOGGER.debug("GET Response: %s %s", response.status_code, response.url)
 
@@ -124,6 +146,7 @@ def post(
             params=params,
             json=body,
             headers=self.request_headers,
+            timeout=self._timeout
         )
         LOGGER.debug("POST Response: %s %s", response.status_code, response.url)
 

argus/client/driver_matrix_tests/client.py

@@ -12,8 +12,10 @@ class Routes(ArgusClient.Routes):
         SUBMIT_DRIVER_FAILURE = "/driver_matrix/result/fail"
         SUBMIT_ENV = "/driver_matrix/env/submit"
 
-    def __init__(self, run_id: UUID, auth_token: str, base_url: str, api_version="v1", extra_headers: dict | None = None) -> None:
-        super().__init__(auth_token, base_url, api_version, extra_headers=extra_headers)
+    def __init__(self, run_id: UUID, auth_token: str, base_url: str, api_version="v1", extra_headers: dict | None = None,
+                 timeout: int = 60, max_retries: int = 3) -> None:
+        super().__init__(auth_token, base_url, api_version, extra_headers=extra_headers,
+                         timeout=timeout, max_retries=max_retries)
         self.run_id = run_id
 
     def submit_driver_matrix_run(self, job_name: str, job_url: str) -> None:

argus/client/generic/client.py

@@ -12,8 +12,10 @@ class ArgusGenericClient(ArgusClient):
     class Routes(ArgusClient.Routes):
         TRIGGER_JOBS = "/planning/plan/trigger"
 
-    def __init__(self, auth_token: str, base_url: str, api_version="v1", extra_headers: dict | None = None) -> None:
-        super().__init__(auth_token, base_url, api_version, extra_headers=extra_headers)
+    def __init__(self, auth_token: str, base_url: str, api_version="v1", extra_headers: dict | None = None,
+                 timeout: int = 180, max_retries: int = 3) -> None:
+        super().__init__(auth_token, base_url, api_version, extra_headers=extra_headers,
+                         timeout=timeout, max_retries=max_retries)
 
     def submit_generic_run(self, build_id: str, run_id: str, started_by: str, build_url: str, sub_type: str = None, scylla_version: str | None = None):
         request_body = {

argus/client/sct/client.py

@@ -30,8 +30,10 @@ class Routes(ArgusClient.Routes):
         SUBMIT_JUNIT_REPORT = "/sct/$id/junit/submit"
         SUBMIT_EMAIL = "/testrun/report/email"
 
-    def __init__(self, run_id: UUID, auth_token: str, base_url: str, api_version="v1", extra_headers: dict | None = None) -> None:
-        super().__init__(auth_token, base_url, api_version, extra_headers=extra_headers)
+    def __init__(self, run_id: UUID, auth_token: str, base_url: str, api_version="v1", extra_headers: dict | None = None,
+                 timeout: int = 60, max_retries: int = 3) -> None:
+        super().__init__(auth_token, base_url, api_version, extra_headers=extra_headers,
+                         timeout=timeout, max_retries=max_retries)
         self.run_id = run_id
 
     def submit_sct_run(self, job_name: str, job_url: str, started_by: str, commit_id: str,

argus/client/sirenada/client.py

@@ -45,9 +45,11 @@ class ArgusSirenadaClient(ArgusClient):
         "skipped": "skipped"
     }
 
-    def __init__(self, auth_token: str, base_url: str, api_version="v1", extra_headers: dict | None = None) -> None:
+    def __init__(self, auth_token: str, base_url: str, api_version="v1", extra_headers: dict | None = None,
+                 timeout: int = 60, max_retries: int = 3) -> None:
         self.results_path: Path | None = None
-        super().__init__(auth_token, base_url, api_version, extra_headers=extra_headers)
+        super().__init__(auth_token, base_url, api_version, extra_headers=extra_headers,
+                         timeout=timeout, max_retries=max_retries)
 
     def _verify_required_files_exist(self, results_path: Path):
         assert (results_path / self._junit_xml_filename).exists(), "Missing jUnit XML results file!"

argus/client/tests/test_timeout_retry.py

@@ -0,0 +1,204 @@
+"""Tests for timeout and retry functionality in ArgusClient."""
+from unittest.mock import patch
+from uuid import uuid4
+
+from argus.client.base import ArgusClient
+from argus.client.sct.client import ArgusSCTClient
+from argus.client.generic.client import ArgusGenericClient
+from argus.client.driver_matrix_tests.client import ArgusDriverMatrixClient
+from argus.client.sirenada.client import ArgusSirenadaClient
+
+
+def test_argus_client_default_timeout():
+    """Test that ArgusClient uses default timeout of 60 seconds."""
+    client = ArgusClient(
+        auth_token="test_token",
+        base_url="https://test.example.com"
+    )
+    assert client._timeout == 60
+
+
+def test_argus_client_custom_timeout():
+    """Test that ArgusClient accepts custom timeout."""
+    client = ArgusClient(
+        auth_token="test_token",
+        base_url="https://test.example.com",
+        timeout=120
+    )
+    assert client._timeout == 120
+
+
+def test_argus_client_session_has_retry_adapter():
+    """Test that ArgusClient session is configured with retry adapter."""
+    client = ArgusClient(
+        auth_token="test_token",
+        base_url="https://test.example.com",
+        max_retries=5
+    )
+
+    # Check that adapters are mounted
+    assert "https://" in client.session.adapters
+    assert "http://" in client.session.adapters
+
+    # Check that the adapter has retry configuration
+    adapter = client.session.get_adapter("https://test.example.com")
+    assert adapter.max_retries.total == 5
+    assert adapter.max_retries.backoff_factor == 1
+    assert adapter.max_retries.status == 0
+    assert adapter.max_retries.status_forcelist == set()
+
+
+def test_get_request_uses_timeout(requests_mock):
+    """Test that GET requests include timeout parameter."""
+    requests_mock.get(
+        "https://test.example.com/api/v1/client/testrun/test-type/test-id/get",
+        json={"status": "ok", "response": {}},
+        status_code=200
+    )
+
+    client = ArgusClient(
+        auth_token="test_token",
+        base_url="https://test.example.com",
+        timeout=30
+    )
+
+    with patch.object(client.session, 'get', wraps=client.session.get) as mock_get:
+        try:
+            client.get(
+                endpoint=ArgusClient.Routes.GET,
+                location_params={"type": "test-type", "id": "test-id"}
+            )
+        except Exception:
+            pass  # We're just checking the call arguments
+
+        # Verify timeout was passed to the request
+        assert mock_get.called
+        call_kwargs = mock_get.call_args.kwargs
+        assert 'timeout' in call_kwargs
+        assert call_kwargs['timeout'] == 30
+
+
+def test_post_request_uses_timeout(requests_mock):
+    """Test that POST requests include timeout parameter."""
+    requests_mock.post(
+        "https://test.example.com/api/v1/client/testrun/test-type/submit",
+        json={"status": "ok"},
+        status_code=200
+    )
+
+    client = ArgusClient(
+        auth_token="test_token",
+        base_url="https://test.example.com",
+        timeout=45
+    )
+
+    with patch.object(client.session, 'post', wraps=client.session.post) as mock_post:
+        try:
+            client.post(
+                endpoint=ArgusClient.Routes.SUBMIT,
+                location_params={"type": "test-type"},
+                body={"test": "data"}
+            )
+        except Exception:
+            pass  # We're just checking the call arguments
+
+        # Verify timeout was passed to the request
+        assert mock_post.called
+        call_kwargs = mock_post.call_args.kwargs
+        assert 'timeout' in call_kwargs
+        assert call_kwargs['timeout'] == 45
+
+
+def test_retry_configuration_is_correct():
+    """Test that the retry adapter is correctly configured."""
+    client = ArgusClient(
+        auth_token="test_token",
+        base_url="https://test.example.com",
+        max_retries=5
+    )
+
+    # Get the adapter
+    adapter = client.session.get_adapter("https://test.example.com")
+
+    # Verify retry configuration
+    assert adapter.max_retries.total == 5
+    assert adapter.max_retries.backoff_factor == 1
+    assert adapter.max_retries.status == 0
+    assert adapter.max_retries.status_forcelist == set()
+    assert "GET" in adapter.max_retries.allowed_methods
+    assert "POST" not in adapter.max_retries.allowed_methods
+
+
+def test_sct_client_passes_timeout_and_retries():
+    """Test that ArgusSCTClient passes timeout and retry parameters to parent."""
+    run_id = uuid4()
+    client = ArgusSCTClient(
+        run_id=run_id,
+        auth_token="test_token",
+        base_url="https://test.example.com",
+        timeout=90,
+        max_retries=5
+    )
+
+    assert client._timeout == 90
+    adapter = client.session.get_adapter("https://test.example.com")
+    assert adapter.max_retries.total == 5
+
+
+def test_generic_client_passes_timeout_and_retries():
+    """Test that ArgusGenericClient passes timeout and retry parameters to parent."""
+    client = ArgusGenericClient(
+        auth_token="test_token",
+        base_url="https://test.example.com",
+        timeout=75,
+        max_retries=4
+    )
+
+    assert client._timeout == 75
+    adapter = client.session.get_adapter("https://test.example.com")
+    assert adapter.max_retries.total == 4
+
+
+def test_driver_matrix_client_passes_timeout_and_retries():
+    """Test that ArgusDriverMatrixClient passes timeout and retry parameters to parent."""
+    run_id = uuid4()
+    client = ArgusDriverMatrixClient(
+        run_id=run_id,
+        auth_token="test_token",
+        base_url="https://test.example.com",
+        timeout=100,
+        max_retries=2
+    )
+
+    assert client._timeout == 100
+    adapter = client.session.get_adapter("https://test.example.com")
+    assert adapter.max_retries.total == 2
+
+
+def test_sirenada_client_passes_timeout_and_retries():
+    """Test that ArgusSirenadaClient passes timeout and retry parameters to parent."""
+    client = ArgusSirenadaClient(
+        auth_token="test_token",
+        base_url="https://test.example.com",
+        timeout=80,
+        max_retries=6
+    )
+
+    assert client._timeout == 80
+    adapter = client.session.get_adapter("https://test.example.com")
+    assert adapter.max_retries.total == 6
+
+
+def test_client_uses_default_values_when_not_specified():
+    """Test that client uses defaults when timeout and retries not specified."""
+    run_id = uuid4()
+    client = ArgusSCTClient(
+        run_id=run_id,
+        auth_token="test_token",
+        base_url="https://test.example.com"
+    )
+
+    # Should use default timeout of 60 and default retries of 3
+    assert client._timeout == 60
+    adapter = client.session.get_adapter("https://test.example.com")
+    assert adapter.max_retries.total == 3

docs/generic_results.md

@@ -38,6 +38,9 @@ Generic results are stored in Argus database and can be accessed via Argus API a
 To send generic results to Argus, you need to create `ResultTable` class basing on `GenericResultTable` available in Argus Client library
 and send it using `ArgusClient` class (or it's child class like `ArgusSCTClient`).
 
+Note: Argus client requests use a 60s timeout by default. `ArgusGenericClient` defaults to 180s to accommodate larger payloads. Only GET
+requests are retried, and only on timeouts; POST submissions surface timeouts to the caller so clients can handle retries explicitly.
+
 Child class of `GenericResultTable` needs to define `Meta` class with `name` and `description` fields. It also needs to define `Columns`
 field that will describe available columns details for given result: as a list of `ColumnMetadata` objects. Each `ColumnMetadata` object
 needs to have `name`, `unit` and `type` and optionally `higher_is_better` fields. `type` field can be one of `ResultType` enum values.

pytest-argus-reporter/pytest_argus_reporter.py

@@ -183,7 +183,11 @@ def __init__(self, config):
 
     @cached_property
     def argus_client(self):
-        return ArgusGenericClient(auth_token=self.api_key, base_url=self.base_url, extra_headers=self.extra_headers)
+        return ArgusGenericClient(
+            auth_token=self.api_key,
+            base_url=self.base_url,
+            extra_headers=self.extra_headers,
+        )
 
     def append_test_data(self, request, test_data):
         self.test_data[request.node.nodeid].update(**test_data)