diff --git a/qase-python-commons/README.md b/qase-python-commons/README.md index 9bf2d0d4..66112cc3 100644 --- a/qase-python-commons/README.md +++ b/qase-python-commons/README.md @@ -400,6 +400,12 @@ When a 429 carries a `Retry-After` header, that value replaces the computed backoff. Qase sends roughly 60 seconds, so a run that hits the rate limit takes longer to finish rather than losing the batch. +The same timeout and retry settings apply to attachment uploads, which go +through the API v1 client in batches of up to 20 files. **If an attachment batch +cannot be uploaded after all attempts**, the reporter logs an error, drops those +attachments and still submits the results they belong to — a failed attachment +never costs you the test results. + **If a batch cannot be delivered after all attempts**, the reporter logs an error naming how many results were lost and **does not mark the run complete**. An open run is the signal that its data is incomplete; a completed run over diff --git a/qase-python-commons/changelog.md b/qase-python-commons/changelog.md index 67a18043..29af66da 100644 --- a/qase-python-commons/changelog.md +++ b/qase-python-commons/changelog.md @@ -1,3 +1,10 @@ +# qase-python-commons@5.1.5 + +## What's new + +- Fixed a hang where an attachment upload could block a reporter thread forever ([#513](https://github.com/qase-tms/qase-python/issues/513)). The configured `testops.api.timeout` was applied to result uploads but never to the API v1 client, so an attachment request that stalled without failing was waited on indefinitely: the batch it belonged to was never submitted, the reporter thread never finished, and `pytest` — one `pytest-xdist` worker in particular — could only be killed by the CI timeout. Every API v1 call now carries the configured timeout. +- Attachment batches are now retried on transient failures using `testops.api.retries` and `testops.api.retryBackoff`, the same policy as result uploads. Previously an attachment error was swallowed inside the upload loop, so the outer retry never saw it. After the attempts are exhausted the reporter logs the failure, drops those attachments and still submits the results — an attachment can no longer cost you the test results. + # qase-python-commons@5.1.4 ## What's new diff --git a/qase-python-commons/pyproject.toml b/qase-python-commons/pyproject.toml index a1c38302..e09e76aa 100644 --- a/qase-python-commons/pyproject.toml +++ b/qase-python-commons/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "qase-python-commons" -version = "5.1.4" +version = "5.1.5" description = "A library for Qase TestOps and Qase Report" readme = "README.md" authors = [{name = "Qase Team", email = "support@qase.io"}] diff --git a/qase-python-commons/src/qase/commons/client/api_v1_client.py b/qase-python-commons/src/qase/commons/client/api_v1_client.py index 09f866be..03520b69 100644 --- a/qase-python-commons/src/qase/commons/client/api_v1_client.py +++ b/qase-python-commons/src/qase/commons/client/api_v1_client.py @@ -8,6 +8,7 @@ from qase.api_client_v1.configuration import Configuration from .. import Logger from .base_api_client import BaseApiClient +from ..retry import send_with_retry from ..exceptions.reporter import ReporterException from ..models import Attachment from ..models.config.framework import Video, Trace @@ -46,7 +47,9 @@ def __init__(self, config: QaseConfig, logger: Logger): def get_project(self, project_code: str) -> Union[Project, None]: try: self.logger.log_debug(f"Getting project {project_code}") - response = ProjectsApi(self.client).get_project(code=project_code) + response = ProjectsApi(self.client).get_project( + code=project_code, _request_timeout=self.config.testops.api.timeout + ) if hasattr(response, 'result'): self.logger.log_debug(f"Project {project_code} found: {response.result.to_json()}") return response.result @@ -59,7 +62,9 @@ def get_environment(self, environment: str, project_code: str) -> Union[str, Non try: self.logger.log_debug(f"Getting environment {environment}") api_instance = EnvironmentsApi(self.client) - response = api_instance.get_environments(code=project_code) + response = api_instance.get_environments( + code=project_code, _request_timeout=self.config.testops.api.timeout + ) if hasattr(response, 'result') and hasattr(response.result, 'entities'): for env in response.result.entities: if env.slug == environment: @@ -76,7 +81,9 @@ def get_configurations(self, project_code: str): try: self.logger.log_debug(f"Getting configurations for project {project_code}") api_instance = ConfigurationsApi(self.client) - response = api_instance.get_configurations(code=project_code) + response = api_instance.get_configurations( + code=project_code, _request_timeout=self.config.testops.api.timeout + ) if hasattr(response, 'result') and hasattr(response.result, 'entities'): return response.result.entities return [] @@ -117,8 +124,9 @@ def find_or_create_configuration(self, project_code: str, config_value: Configur # Create new group group_create = ConfigurationGroupCreate(title=config_value.name) group_response = ConfigurationsApi(self.client).create_configuration_group( - code=project_code, - configuration_group_create=group_create + code=project_code, + configuration_group_create=group_create, + _request_timeout=self.config.testops.api.timeout ) group_id = group_response.result.id @@ -129,7 +137,8 @@ def find_or_create_configuration(self, project_code: str, config_value: Configur ) config_response = ConfigurationsApi(self.client).create_configuration( code=project_code, - configuration_create=config_create + configuration_create=config_create, + _request_timeout=self.config.testops.api.timeout ) config_id = config_response.result.id return config_id @@ -141,12 +150,16 @@ def find_or_create_configuration(self, project_code: str, config_value: Configur def complete_run(self, project_code: str, run_id: int) -> None: api_runs = RunsApi(self.client) self.logger.log_debug(f"Completing run {run_id}") - res = api_runs.get_run(project_code, run_id).result + res = api_runs.get_run( + project_code, run_id, _request_timeout=self.config.testops.api.timeout + ).result if res.status == 1: self.logger.log_debug(f"Run {run_id} already completed") return try: - api_runs.complete_run(project_code, run_id) + api_runs.complete_run( + project_code, run_id, _request_timeout=self.config.testops.api.timeout + ) self.logger.log(f"Test run link: {self.web}/run/{project_code}/dashboard/{run_id}", "info") except Exception as e: self.logger.log(f"Error at completing run {run_id}: {e}", "error") @@ -240,10 +253,33 @@ def _upload_attachment(self, project_code: str, attachment: Union[Attachment, Li # Prepare files for upload files_for_upload = [att.get_for_upload() for att in batch] - - # Upload batch - response = attach_api.upload_attachment(project_code, file=files_for_upload) - + + # The timeout is what keeps a stalled connection from blocking + # the reporter thread forever: without it urllib3 waits + # indefinitely, no exception is ever raised, and the retry below + # never gets a chance to run. + # + # Unlike results, an attachment carries no idempotency key, so a + # retry whose first response was lost can leave an unreferenced + # file behind. That is cheaper than the alternative: the whole + # batch of results these attachments belong to going nowhere. + response = None + + def upload_batch(): + nonlocal response + response = attach_api.upload_attachment( + project_code, + file=files_for_upload, + _request_timeout=self.config.testops.api.timeout + ) + + send_with_retry( + upload_batch, + attempts=self.config.testops.api.retries, + backoff=self.config.testops.api.retry_backoff, + logger=self.logger, + ) + if response.result: all_uploaded.extend(response.result) self.logger.log_debug( @@ -294,7 +330,8 @@ def create_test_run(self, project_code: str, title: str, description: str, plan_ try: result = RunsApi(self.client).create_run( code=project_code, - run_create=RunCreate(**{k: v for k, v in kwargs.items() if v is not None}) + run_create=RunCreate(**{k: v for k, v in kwargs.items() if v is not None}), + _request_timeout=self.config.testops.api.timeout ) run_id = result.result.id @@ -311,7 +348,9 @@ def create_test_run(self, project_code: str, title: str, description: str, plan_ def check_test_run(self, project_code: str, run_id: int) -> bool: api_runs = RunsApi(self.client) - run = api_runs.get_run(code=project_code, id=run_id) + run = api_runs.get_run( + code=project_code, id=run_id, _request_timeout=self.config.testops.api.timeout + ) if run.result.id: return True return False @@ -332,7 +371,10 @@ def enable_public_report(self, project_code: str, run_id: int) -> str: run_public = RunPublic(status=True) # Call the API to enable public report - response = api_runs.update_run_publicity(project_code, run_id, run_public) + response = api_runs.update_run_publicity( + project_code, run_id, run_public, + _request_timeout=self.config.testops.api.timeout + ) # Extract the public URL from response if response.result and response.result.url: @@ -368,7 +410,8 @@ def update_external_link(self, project_code: str, run_id: int): RunsApi(self.client).run_update_external_issue( code=project_code, - runexternal_issues=run_external_issues + runexternal_issues=run_external_issues, + _request_timeout=self.config.testops.api.timeout ) self.logger.log(f"External link updated for run {run_id}: {external_link.link}", "debug") diff --git a/qase-python-commons/tests/tests_qase_commons/test_api_v1_client.py b/qase-python-commons/tests/tests_qase_commons/test_api_v1_client.py new file mode 100644 index 00000000..4691914c --- /dev/null +++ b/qase-python-commons/tests/tests_qase_commons/test_api_v1_client.py @@ -0,0 +1,210 @@ +from unittest.mock import Mock, patch + +import pytest + +from qase.commons import retry as retry_module +from qase.commons.client.api_v1_client import ApiV1Client +from qase.commons.models.attachment import Attachment + + +def _client(timeout: int = 7, retries: int = 3, retry_backoff: int = 1) -> ApiV1Client: + """An ApiV1Client whose __init__ is bypassed. + + The real __init__ builds an API client and reads certifi; the methods under + test only need self.client, self.config and self.logger. + """ + client = ApiV1Client.__new__(ApiV1Client) + config = Mock() + config.testops.api.timeout = timeout + config.testops.api.retries = retries + config.testops.api.retry_backoff = retry_backoff + config.testops.configurations.values = [] + config.testops.run.tags = [] + config.testops.run.external_link = None + client.config = config + client.logger = Mock() + client.client = Mock() + client.web = "https://app.qase.io" + return client + + +def _attachment(name: str = "log.txt") -> Attachment: + return Attachment(file_name=name, mime_type="text/plain", content="content") + + +def _http_error(status: int) -> Exception: + error = Exception(f"HTTP {status}") + error.status = status + return error + + +@pytest.fixture(autouse=True) +def _no_backoff_sleep(): + """Keep the retry ladder instant without faking the retry logic itself.""" + with patch.object(retry_module.time, "sleep"): + yield + + +def test_upload_attachment_passes_the_configured_timeout(): + client = _client(timeout=7) + + with patch("qase.commons.client.api_v1_client.AttachmentsApi") as attachments_api: + client._upload_attachment("DEMO", [_attachment()]) + + kwargs = attachments_api.return_value.upload_attachment.call_args.kwargs + assert kwargs["_request_timeout"] == 7 + + +def test_upload_attachment_retries_a_retryable_failure(): + client = _client(retries=3) + uploaded = Mock() + response = Mock(result=[uploaded]) + + with patch("qase.commons.client.api_v1_client.AttachmentsApi") as attachments_api: + attachments_api.return_value.upload_attachment.side_effect = [ + TimeoutError("read timed out"), + response, + ] + result = client._upload_attachment("DEMO", [_attachment()]) + + assert attachments_api.return_value.upload_attachment.call_count == 2 + assert result == [uploaded] + + +def test_upload_attachment_stops_after_the_configured_attempts(): + client = _client(retries=2) + + with patch("qase.commons.client.api_v1_client.AttachmentsApi") as attachments_api: + attachments_api.return_value.upload_attachment.side_effect = TimeoutError( + "read timed out" + ) + result = client._upload_attachment("DEMO", [_attachment()]) + + assert attachments_api.return_value.upload_attachment.call_count == 2 + assert result == [] + + +def test_upload_attachment_does_not_retry_a_non_retryable_failure(): + client = _client(retries=3) + + with patch("qase.commons.client.api_v1_client.AttachmentsApi") as attachments_api: + attachments_api.return_value.upload_attachment.side_effect = _http_error(400) + result = client._upload_attachment("DEMO", [_attachment()]) + + assert attachments_api.return_value.upload_attachment.call_count == 1 + assert result == [] + + +def test_upload_attachment_keeps_uploading_after_a_batch_is_exhausted(): + """A batch that cannot be uploaded must not abort the remaining batches.""" + client = _client(retries=2) + uploaded = Mock() + # 21 attachments do not fit into one request (20 files max), so they are + # split into two batches; the first one fails for good. + attachments = [_attachment(f"log-{i}.txt") for i in range(21)] + + with patch("qase.commons.client.api_v1_client.AttachmentsApi") as attachments_api: + attachments_api.return_value.upload_attachment.side_effect = [ + TimeoutError("read timed out"), + TimeoutError("read timed out"), + Mock(result=[uploaded]), + ] + result = client._upload_attachment("DEMO", attachments) + + assert result == [uploaded] + + +def test_get_project_passes_the_configured_timeout(): + client = _client(timeout=7) + + with patch("qase.commons.client.api_v1_client.ProjectsApi") as projects_api: + client.get_project("DEMO") + + kwargs = projects_api.return_value.get_project.call_args.kwargs + assert kwargs["_request_timeout"] == 7 + + +def test_get_environment_passes_the_configured_timeout(): + client = _client(timeout=7) + + with patch("qase.commons.client.api_v1_client.EnvironmentsApi") as environments_api: + environments_api.return_value.get_environments.return_value = Mock( + result=Mock(entities=[]) + ) + client.get_environment("staging", "DEMO") + + kwargs = environments_api.return_value.get_environments.call_args.kwargs + assert kwargs["_request_timeout"] == 7 + + +def test_get_configurations_passes_the_configured_timeout(): + client = _client(timeout=7) + + with patch("qase.commons.client.api_v1_client.ConfigurationsApi") as configurations_api: + configurations_api.return_value.get_configurations.return_value = Mock( + result=Mock(entities=[]) + ) + client.get_configurations("DEMO") + + kwargs = configurations_api.return_value.get_configurations.call_args.kwargs + assert kwargs["_request_timeout"] == 7 + + +def test_create_test_run_passes_the_configured_timeout(): + client = _client(timeout=7) + + with patch("qase.commons.client.api_v1_client.RunsApi") as runs_api: + client.create_test_run("DEMO", "title", "description") + + kwargs = runs_api.return_value.create_run.call_args.kwargs + assert kwargs["_request_timeout"] == 7 + + +def test_check_test_run_passes_the_configured_timeout(): + client = _client(timeout=7) + + with patch("qase.commons.client.api_v1_client.RunsApi") as runs_api: + client.check_test_run("DEMO", 1) + + kwargs = runs_api.return_value.get_run.call_args.kwargs + assert kwargs["_request_timeout"] == 7 + + +def test_complete_run_passes_the_configured_timeout(): + client = _client(timeout=7) + + with patch("qase.commons.client.api_v1_client.RunsApi") as runs_api: + runs_api.return_value.get_run.return_value = Mock(result=Mock(status=0)) + client.complete_run("DEMO", 1) + + kwargs = runs_api.return_value.complete_run.call_args.kwargs + assert kwargs["_request_timeout"] == 7 + + +def test_enable_public_report_passes_the_configured_timeout(): + client = _client(timeout=7) + + with patch("qase.commons.client.api_v1_client.RunsApi") as runs_api: + client.enable_public_report("DEMO", 1) + + kwargs = runs_api.return_value.update_run_publicity.call_args.kwargs + assert kwargs["_request_timeout"] == 7 + + +def test_find_or_create_configuration_passes_the_configured_timeout(): + client = _client(timeout=7) + client.config.testops.configurations.create_if_not_exists = True + client.get_configurations = Mock(return_value=[]) + config_value = Mock(name_="group", value="value") + config_value.name = "group" + + with patch("qase.commons.client.api_v1_client.ConfigurationsApi") as configurations_api: + configurations_api.return_value.create_configuration_group.return_value = Mock( + result=Mock(id=1) + ) + client.find_or_create_configuration("DEMO", config_value) + + group_kwargs = configurations_api.return_value.create_configuration_group.call_args.kwargs + config_kwargs = configurations_api.return_value.create_configuration.call_args.kwargs + assert group_kwargs["_request_timeout"] == 7 + assert config_kwargs["_request_timeout"] == 7