From ca0325cb1ccbe37f78599ee61bce1b1f20ecdbc2 Mon Sep 17 00:00:00 2001 From: Andreas Kunft Date: Tue, 5 May 2026 13:33:44 +0200 Subject: [PATCH 1/6] feat: expose custom_metadata_checksum on update_custom_metadata and update_item_custom_metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds optional optimistic concurrency control to run and item custom metadata updates. When `custom_metadata_checksum` is provided the server rejects the write with HTTP 412 if the metadata was modified since the checksum was read; pass `None` (default) to skip the check. Changes: - platform/resources/runs.py: add keyword-only `custom_metadata_checksum` param to `Run.update_custom_metadata` and `Run.update_item_custom_metadata`, forwarded to `CustomMetadataUpdateRequest` - application/_service.py: propagate `custom_metadata_checksum` through `ApplicationService.application_run_update_custom_metadata`, `application_run_update_item_custom_metadata`, and both static wrappers; handle HTTP 412 explicitly as `ValueError` with a clear concurrency-conflict message instead of the generic `RuntimeError` fallback - tests: add unit tests for checksum forwarding (None and non-None) at the `Run` resource layer; add service-layer tests for checksum propagation and 412→ValueError mapping; fix stale assertions to include `custom_metadata_checksum=None` --- src/aignostics/application/_cli.py | 38 +++++++++- src/aignostics/application/_service.py | 57 ++++++++++++++- src/aignostics/platform/resources/runs.py | 18 ++++- src/aignostics/system/_exceptions.py | 9 +++ tests/aignostics/application/service_test.py | 77 +++++++++++++++++++- 5 files changed, 190 insertions(+), 9 deletions(-) diff --git a/src/aignostics/application/_cli.py b/src/aignostics/application/_cli.py index 9ee527859..dcc877f86 100644 --- a/src/aignostics/application/_cli.py +++ b/src/aignostics/application/_cli.py @@ -1213,10 +1213,22 @@ def run_update_metadata( metadata_json: Annotated[ str, typer.Argument(..., help='Custom metadata as JSON string (e.g., \'{"key": "value"}\')') ], + checksum: Annotated[ + str | None, + typer.Option( + "--checksum", + help=( + "Optional checksum for optimistic concurrency control. " + "The server rejects the update with HTTP 412 if the metadata was modified since this checksum was read." + ), + ), + ] = None, ) -> None: """Update custom metadata for a run.""" import json # noqa: PLC0415 + from aignostics.system._exceptions import ConcurrencyConflictError # noqa: PLC0415 + logger.trace("Updating custom metadata for run with ID '{}'", run_id) try: @@ -1230,13 +1242,17 @@ def run_update_metadata( console.print(f"[error]Error:[/error] Invalid JSON: {e}") sys.exit(1) - Service().application_run_update_custom_metadata(run_id, custom_metadata) + Service().application_run_update_custom_metadata(run_id, custom_metadata, custom_metadata_checksum=checksum) logger.debug("Updated custom metadata for run with ID '{}'.", run_id) console.print(f"Successfully updated custom metadata for run with ID '{run_id}'.") except NotFoundException: logger.warning(f"Run with ID '{run_id}' not found.") console.print(f"[warning]Warning:[/warning] Run with ID '{run_id}' not found.") sys.exit(2) + except ConcurrencyConflictError as e: + logger.warning(f"Concurrency conflict updating metadata for run '{run_id}': {e}") + console.print(f"[warning]Warning:[/warning] Metadata was modified by another process. Re-read and retry: {e}") + sys.exit(2) except ValueError as e: logger.warning(f"Run ID '{run_id}' invalid or metadata invalid: {e}") console.print(f"[warning]Warning:[/warning] Run ID '{run_id}' invalid or metadata invalid: {e}") @@ -1254,10 +1270,22 @@ def run_update_item_metadata( metadata_json: Annotated[ str, typer.Argument(..., help='Custom metadata as JSON string (e.g., \'{"key": "value"}\')') ], + checksum: Annotated[ + str | None, + typer.Option( + "--checksum", + help=( + "Optional checksum for optimistic concurrency control. " + "The server rejects the update with HTTP 412 if the metadata was modified since this checksum was read." + ), + ), + ] = None, ) -> None: """Update custom metadata for an item in a run.""" import json # noqa: PLC0415 + from aignostics.system._exceptions import ConcurrencyConflictError # noqa: PLC0415 + logger.trace("Updating custom metadata for item '{}' in run with ID '{}'", external_id, run_id) try: @@ -1271,13 +1299,19 @@ def run_update_item_metadata( console.print(f"[error]Error:[/error] Invalid JSON: {e}") sys.exit(1) - Service().application_run_update_item_custom_metadata(run_id, external_id, custom_metadata) + Service().application_run_update_item_custom_metadata( + run_id, external_id, custom_metadata, custom_metadata_checksum=checksum + ) logger.debug("Updated custom metadata for item '{}' in run with ID '{}'.", external_id, run_id) console.print(f"Successfully updated custom metadata for item '{external_id}' in run with ID '{run_id}'.") except NotFoundException: logger.warning(f"Run with ID '{run_id}' or item '{external_id}' not found.") console.print(f"[warning]Warning:[/warning] Run with ID '{run_id}' or item '{external_id}' not found.") sys.exit(2) + except ConcurrencyConflictError as e: + logger.warning("Concurrency conflict updating metadata for item '{}' in run '{}': {}", external_id, run_id, e) + console.print(f"[warning]Warning:[/warning] Metadata was modified by another process. Re-read and retry: {e}") + sys.exit(2) except ValueError as e: logger.warning( "Run ID '{}' or item external ID '{}' invalid or metadata invalid: {}", diff --git a/src/aignostics/application/_service.py b/src/aignostics/application/_service.py index 315c674f6..d6295a1b4 100644 --- a/src/aignostics/application/_service.py +++ b/src/aignostics/application/_service.py @@ -33,6 +33,7 @@ RunState, ) from aignostics.platform import Service as PlatformService +from aignostics.system._exceptions import ConcurrencyConflictError from aignostics.utils import BaseService, Health, sanitize_path_component from aignostics.wsi import Service as WSIService @@ -1120,21 +1121,31 @@ def application_run_update_custom_metadata( self, run_id: str, custom_metadata: dict[str, Any], + *, + custom_metadata_checksum: str | None = None, ) -> None: """Update custom metadata for an existing application run. Args: run_id (str): The ID of the run to update custom_metadata (dict[str, Any]): The new custom metadata to attach to the run. + custom_metadata_checksum (str | None): Optional checksum for optimistic concurrency + control. When provided, the server returns HTTP 412 (and rejects the update) if + the metadata was modified since the checksum was read. Pass ``None`` to skip + the precondition check. Raises: NotFoundException: If the application run with the given ID is not found. + ConcurrencyConflictError: If the checksum precondition failed (HTTP 412). ValueError: If the run ID is invalid. RuntimeError: If updating the run metadata fails unexpectedly. """ try: logger.trace("Updating custom metadata for run with ID '{}'", run_id) - self._get_platform_client().run(run_id).update_custom_metadata(custom_metadata) + self._get_platform_client().run(run_id).update_custom_metadata( + custom_metadata, + custom_metadata_checksum=custom_metadata_checksum, + ) logger.trace("Updated custom metadata for run with ID '{}'", run_id) except ValueError as e: message = f"Failed to update custom metadata for run with ID '{run_id}': ValueError {e}" @@ -1145,6 +1156,13 @@ def application_run_update_custom_metadata( logger.warning(message) raise NotFoundException(message) from e except ApiException as e: + if e.status == HTTPStatus.PRECONDITION_FAILED: + message = ( + f"Custom metadata for run '{run_id}' was modified since the checksum was read " + f"(optimistic concurrency conflict): {e!s}." + ) + logger.warning(message) + raise ConcurrencyConflictError(message) from e if e.status == HTTPStatus.UNPROCESSABLE_ENTITY: message = f"Run ID '{run_id}' invalid: {e!s}." logger.warning(message) @@ -1161,25 +1179,36 @@ def application_run_update_custom_metadata( def application_run_update_custom_metadata_static( run_id: str, custom_metadata: dict[str, Any], + *, + custom_metadata_checksum: str | None = None, ) -> None: """Static wrapper for updating custom metadata for an application run. Args: run_id (str): The ID of the run to update custom_metadata (dict[str, Any]): The new custom metadata to attach to the run. + custom_metadata_checksum (str | None): Optional checksum for optimistic concurrency + control. When provided, the server returns HTTP 412 (and rejects the update) if + the metadata was modified since the checksum was read. Pass ``None`` to skip + the precondition check. Raises: NotFoundException: If the application run with the given ID is not found. + ConcurrencyConflictError: If the checksum precondition failed (HTTP 412). ValueError: If the run ID is invalid. RuntimeError: If updating the run metadata fails unexpectedly. """ - Service().application_run_update_custom_metadata(run_id, custom_metadata) + Service().application_run_update_custom_metadata( + run_id, custom_metadata, custom_metadata_checksum=custom_metadata_checksum + ) def application_run_update_item_custom_metadata( self, run_id: str, external_id: str, custom_metadata: dict[str, Any], + *, + custom_metadata_checksum: str | None = None, ) -> None: """Update custom metadata for an existing item in an application run. @@ -1187,9 +1216,14 @@ def application_run_update_item_custom_metadata( run_id (str): The ID of the run containing the item external_id (str): The external ID of the item to update custom_metadata (dict[str, Any]): The new custom metadata to attach to the item. + custom_metadata_checksum (str | None): Optional checksum for optimistic concurrency + control. When provided, the server returns HTTP 412 (and rejects the update) if + the metadata was modified since the checksum was read. Pass ``None`` to skip + the precondition check. Raises: NotFoundException: If the application run or item with the given IDs is not found. + ConcurrencyConflictError: If the checksum precondition failed (HTTP 412). ValueError: If the run ID or item external ID is invalid. RuntimeError: If updating the item metadata fails unexpectedly. """ @@ -1202,6 +1236,7 @@ def application_run_update_item_custom_metadata( self._get_platform_client().run(run_id).update_item_custom_metadata( external_id, custom_metadata, + custom_metadata_checksum=custom_metadata_checksum, ) logger.trace( "Updated custom metadata for item '{}' in run with ID '{}'", @@ -1219,6 +1254,13 @@ def application_run_update_item_custom_metadata( logger.warning(message) raise NotFoundException(message) from e except ApiException as e: + if e.status == HTTPStatus.PRECONDITION_FAILED: + message = ( + f"Custom metadata for item '{external_id}' in run '{run_id}' was modified since " + f"the checksum was read (optimistic concurrency conflict): {e!s}." + ) + logger.warning(message) + raise ConcurrencyConflictError(message) from e if e.status == HTTPStatus.UNPROCESSABLE_ENTITY: message = f"Run ID '{run_id}' or item external ID '{external_id}' invalid: {e!s}." logger.warning(message) @@ -1236,6 +1278,8 @@ def application_run_update_item_custom_metadata_static( run_id: str, external_id: str, custom_metadata: dict[str, Any], + *, + custom_metadata_checksum: str | None = None, ) -> None: """Static wrapper for updating custom metadata for an item in an application run. @@ -1243,13 +1287,20 @@ def application_run_update_item_custom_metadata_static( run_id (str): The ID of the run containing the item external_id (str): The external ID of the item to update custom_metadata (dict[str, Any]): The new custom metadata to attach to the item. + custom_metadata_checksum (str | None): Optional checksum for optimistic concurrency + control. When provided, the server returns HTTP 412 (and rejects the update) if + the metadata was modified since the checksum was read. Pass ``None`` to skip + the precondition check. Raises: NotFoundException: If the application run or item with the given IDs is not found. + ConcurrencyConflictError: If the checksum precondition failed (HTTP 412). ValueError: If the run ID or item external ID is invalid. RuntimeError: If updating the item metadata fails unexpectedly. """ - Service().application_run_update_item_custom_metadata(run_id, external_id, custom_metadata) + Service().application_run_update_item_custom_metadata( + run_id, external_id, custom_metadata, custom_metadata_checksum=custom_metadata_checksum + ) def application_run_cancel(self, run_id: str) -> None: """Cancel a run by its ID. diff --git a/src/aignostics/platform/resources/runs.py b/src/aignostics/platform/resources/runs.py index 7a861c4b4..a95623adb 100644 --- a/src/aignostics/platform/resources/runs.py +++ b/src/aignostics/platform/resources/runs.py @@ -596,11 +596,17 @@ def ensure_artifacts_downloaded( def update_custom_metadata( self, custom_metadata: dict[str, Any], + *, + custom_metadata_checksum: str | None = None, ) -> None: """Update custom metadata for this application run. Args: custom_metadata (dict[str, Any]): The new custom metadata to attach to the run. + custom_metadata_checksum (str | None): Optional checksum for optimistic concurrency + control. When provided, the server returns HTTP 412 (and rejects the update) if + the metadata was modified since the checksum was read. Pass ``None`` to skip + the precondition check. Raises: Exception: If the API request fails. @@ -615,7 +621,8 @@ def update_custom_metadata( self._api.put_run_custom_metadata_v1_runs_run_id_custom_metadata_put( self.run_id, custom_metadata_update_request=CustomMetadataUpdateRequest( - custom_metadata=cast("dict[str, Any]", convert_to_json_serializable(custom_metadata)) + custom_metadata=cast("dict[str, Any]", convert_to_json_serializable(custom_metadata)), + custom_metadata_checksum=custom_metadata_checksum, ), _request_timeout=settings().run_submit_timeout, _headers={"User-Agent": user_agent()}, @@ -626,12 +633,18 @@ def update_item_custom_metadata( self, external_id: str, custom_metadata: dict[str, Any], + *, + custom_metadata_checksum: str | None = None, ) -> None: """Update custom metadata for an item in this application run. Args: external_id (str): The external ID of the item. custom_metadata (dict[str, Any]): The new custom metadata to attach to the item. + custom_metadata_checksum (str | None): Optional checksum for optimistic concurrency + control. When provided, the server returns HTTP 412 (and rejects the update) if + the metadata was modified since the checksum was read. Pass ``None`` to skip + the precondition check. Raises: Exception: If the API request fails. @@ -647,7 +660,8 @@ def update_item_custom_metadata( self.run_id, external_id, custom_metadata_update_request=CustomMetadataUpdateRequest( - custom_metadata=cast("dict[str, Any]", convert_to_json_serializable(custom_metadata)) + custom_metadata=cast("dict[str, Any]", convert_to_json_serializable(custom_metadata)), + custom_metadata_checksum=custom_metadata_checksum, ), _request_timeout=settings().run_submit_timeout, _headers={"User-Agent": user_agent()}, diff --git a/src/aignostics/system/_exceptions.py b/src/aignostics/system/_exceptions.py index e0a200463..9e878c921 100644 --- a/src/aignostics/system/_exceptions.py +++ b/src/aignostics/system/_exceptions.py @@ -7,3 +7,12 @@ class OpenAPISchemaError(ValueError): def __init__(self, error: Exception) -> None: """Initialize exception with the underlying error.""" super().__init__(f"Failed to load OpenAPI schema: {error}") + + +class ConcurrencyConflictError(ValueError): + """Raised when an optimistic concurrency precondition (HTTP 412) fails. + + Subclasses ValueError so existing ``except ValueError`` callers still catch it, + while callers that need to distinguish a conflict from a bad-ID error can use + ``except ConcurrencyConflictError``. + """ diff --git a/tests/aignostics/application/service_test.py b/tests/aignostics/application/service_test.py index ea023455b..e2234cbd0 100644 --- a/tests/aignostics/application/service_test.py +++ b/tests/aignostics/application/service_test.py @@ -1,6 +1,7 @@ """Tests to verify the service functionality of the application module.""" from datetime import UTC, datetime, timedelta +from http import HTTPStatus from unittest.mock import MagicMock, patch import pytest @@ -8,6 +9,8 @@ from aignostics.application import Service as ApplicationService from aignostics.platform import NotFoundException, RunData, RunOutput +from aignostics.platform.resources.runs import ApiException +from aignostics.system._exceptions import ConcurrencyConflictError from tests.constants_test import ( HETA_APPLICATION_ID, HETA_APPLICATION_VERSION, @@ -511,7 +514,7 @@ def test_application_run_update_custom_metadata_success(mock_get_client: MagicMo # Verify the run() method was called with correct run_id mock_client.run.assert_called_once_with("run-123") # Verify the update_custom_metadata method was called with correct arguments - mock_run.update_custom_metadata.assert_called_once_with(custom_metadata) + mock_run.update_custom_metadata.assert_called_once_with(custom_metadata, custom_metadata_checksum=None) @pytest.mark.unit @@ -548,7 +551,9 @@ def test_application_run_update_item_custom_metadata_success(mock_get_client: Ma # Verify the run() method was called with correct run_id mock_client.run.assert_called_once_with("run-123") # Verify the update_item_custom_metadata method was called with correct arguments - mock_run.update_item_custom_metadata.assert_called_once_with("item-ext-id", custom_metadata) + mock_run.update_item_custom_metadata.assert_called_once_with( + "item-ext-id", custom_metadata, custom_metadata_checksum=None + ) @pytest.mark.unit @@ -565,3 +570,71 @@ def test_application_run_update_item_custom_metadata_not_found(mock_get_client: with pytest.raises(NotFoundException, match="not found"): service.application_run_update_item_custom_metadata("run-123", "invalid-item-id", {"key": "value"}) + + +@pytest.mark.unit +@patch("aignostics.application._service.Service._get_platform_client") +def test_application_run_update_custom_metadata_with_checksum(mock_get_client: MagicMock) -> None: + """Checksum is forwarded through the service layer to Run.update_custom_metadata.""" + mock_client = MagicMock() + mock_run = MagicMock() + mock_client.run.return_value = mock_run + mock_get_client.return_value = mock_client + + service = ApplicationService() + service.application_run_update_custom_metadata("run-123", {"key": "value"}, custom_metadata_checksum="abc123") + + mock_run.update_custom_metadata.assert_called_once_with({"key": "value"}, custom_metadata_checksum="abc123") + + +@pytest.mark.unit +@patch("aignostics.application._service.Service._get_platform_client") +def test_application_run_update_custom_metadata_412_raises_concurrency_error(mock_get_client: MagicMock) -> None: + """HTTP 412 from server (optimistic concurrency conflict) raises ConcurrencyConflictError.""" + mock_client = MagicMock() + mock_run = MagicMock() + mock_run.update_custom_metadata.side_effect = ApiException(status=HTTPStatus.PRECONDITION_FAILED) + mock_client.run.return_value = mock_run + mock_get_client.return_value = mock_client + + service = ApplicationService() + + with pytest.raises(ConcurrencyConflictError, match="optimistic concurrency conflict"): + service.application_run_update_custom_metadata("run-123", {"key": "value"}, custom_metadata_checksum="stale") + + +@pytest.mark.unit +@patch("aignostics.application._service.Service._get_platform_client") +def test_application_run_update_item_custom_metadata_with_checksum(mock_get_client: MagicMock) -> None: + """Checksum is forwarded through the service layer to Run.update_item_custom_metadata.""" + mock_client = MagicMock() + mock_run = MagicMock() + mock_client.run.return_value = mock_run + mock_get_client.return_value = mock_client + + service = ApplicationService() + service.application_run_update_item_custom_metadata( + "run-123", "item-ext-id", {"key": "value"}, custom_metadata_checksum="abc123" + ) + + mock_run.update_item_custom_metadata.assert_called_once_with( + "item-ext-id", {"key": "value"}, custom_metadata_checksum="abc123" + ) + + +@pytest.mark.unit +@patch("aignostics.application._service.Service._get_platform_client") +def test_application_run_update_item_custom_metadata_412_raises_concurrency_error(mock_get_client: MagicMock) -> None: + """HTTP 412 from server raises ConcurrencyConflictError for item metadata update.""" + mock_client = MagicMock() + mock_run = MagicMock() + mock_run.update_item_custom_metadata.side_effect = ApiException(status=HTTPStatus.PRECONDITION_FAILED) + mock_client.run.return_value = mock_run + mock_get_client.return_value = mock_client + + service = ApplicationService() + + with pytest.raises(ConcurrencyConflictError, match="optimistic concurrency conflict"): + service.application_run_update_item_custom_metadata( + "run-123", "item-ext-id", {"key": "value"}, custom_metadata_checksum="stale" + ) From b58c3029823d7afcb3153461f17071790a35cb1a Mon Sep 17 00:00:00 2001 From: Andreas Kunft Date: Thu, 7 May 2026 12:50:31 +0200 Subject: [PATCH 2/6] fix: address Copilot review comments on checksum-param PR - Re-export ConcurrencyConflictError from aignostics.system public API - Update imports in _service.py and _cli.py to use public path - Add unit tests for CLI --checksum success path and ConcurrencyConflictError handler on both update-metadata and update-item-metadata commands --- src/aignostics/application/_cli.py | 4 +- src/aignostics/application/_service.py | 2 +- src/aignostics/system/__init__.py | 2 + tests/aignostics/application/cli_test.py | 90 ++++++++++++++++++++++++ 4 files changed, 95 insertions(+), 3 deletions(-) diff --git a/src/aignostics/application/_cli.py b/src/aignostics/application/_cli.py index dcc877f86..5b0b463a8 100644 --- a/src/aignostics/application/_cli.py +++ b/src/aignostics/application/_cli.py @@ -1227,7 +1227,7 @@ def run_update_metadata( """Update custom metadata for a run.""" import json # noqa: PLC0415 - from aignostics.system._exceptions import ConcurrencyConflictError # noqa: PLC0415 + from aignostics.system import ConcurrencyConflictError # noqa: PLC0415 logger.trace("Updating custom metadata for run with ID '{}'", run_id) @@ -1284,7 +1284,7 @@ def run_update_item_metadata( """Update custom metadata for an item in a run.""" import json # noqa: PLC0415 - from aignostics.system._exceptions import ConcurrencyConflictError # noqa: PLC0415 + from aignostics.system import ConcurrencyConflictError # noqa: PLC0415 logger.trace("Updating custom metadata for item '{}' in run with ID '{}'", external_id, run_id) diff --git a/src/aignostics/application/_service.py b/src/aignostics/application/_service.py index d6295a1b4..dbfc1b679 100644 --- a/src/aignostics/application/_service.py +++ b/src/aignostics/application/_service.py @@ -33,7 +33,7 @@ RunState, ) from aignostics.platform import Service as PlatformService -from aignostics.system._exceptions import ConcurrencyConflictError +from aignostics.system import ConcurrencyConflictError from aignostics.utils import BaseService, Health, sanitize_path_component from aignostics.wsi import Service as WSIService diff --git a/src/aignostics/system/__init__.py b/src/aignostics/system/__init__.py index 1dab2d033..0397c147e 100644 --- a/src/aignostics/system/__init__.py +++ b/src/aignostics/system/__init__.py @@ -1,10 +1,12 @@ """System module.""" from ._cli import cli +from ._exceptions import ConcurrencyConflictError from ._service import Service from ._settings import Settings __all__ = [ + "ConcurrencyConflictError", "Service", "Settings", "cli", diff --git a/tests/aignostics/application/cli_test.py b/tests/aignostics/application/cli_test.py index 4c5b9aeb0..6dbba0fa2 100644 --- a/tests/aignostics/application/cli_test.py +++ b/tests/aignostics/application/cli_test.py @@ -1211,6 +1211,96 @@ def test_cli_run_update_item_metadata_not_dict(runner: CliRunner) -> None: assert "Metadata must be a JSON object" in result.output +@pytest.mark.unit +def test_cli_run_update_metadata_success_with_checksum(runner: CliRunner) -> None: + """Check run update-metadata command succeeds and forwards --checksum to the service.""" + with patch("aignostics.application._cli.Service") as mock_service_cls: + result = runner.invoke( + cli, + [ + "application", + "run", + "update-metadata", + "run-123", + '{"key": "value"}', + "--checksum", + "abc123", + ], + ) + assert result.exit_code == 0 + assert "Successfully updated" in result.output + mock_service_cls.return_value.application_run_update_custom_metadata.assert_called_once_with( + "run-123", {"key": "value"}, custom_metadata_checksum="abc123" + ) + + +@pytest.mark.unit +def test_cli_run_update_metadata_concurrency_conflict(runner: CliRunner) -> None: + """Check run update-metadata exits 2 with a clear message on ConcurrencyConflictError.""" + from aignostics.system import ConcurrencyConflictError + + with patch("aignostics.application._cli.Service") as mock_service_cls: + mock_service_cls.return_value.application_run_update_custom_metadata.side_effect = ConcurrencyConflictError( + "stale checksum" + ) + result = runner.invoke( + cli, + ["application", "run", "update-metadata", "run-123", '{"key": "value"}', "--checksum", "old"], + ) + assert result.exit_code == 2 + assert "modified by another process" in result.output + + +@pytest.mark.unit +def test_cli_run_update_item_metadata_success_with_checksum(runner: CliRunner) -> None: + """Check run update-item-metadata command succeeds and forwards --checksum to the service.""" + with patch("aignostics.application._cli.Service") as mock_service_cls: + result = runner.invoke( + cli, + [ + "application", + "run", + "update-item-metadata", + "run-123", + "item-ext-id", + '{"key": "value"}', + "--checksum", + "abc123", + ], + ) + assert result.exit_code == 0 + assert "Successfully updated" in result.output + mock_service_cls.return_value.application_run_update_item_custom_metadata.assert_called_once_with( + "run-123", "item-ext-id", {"key": "value"}, custom_metadata_checksum="abc123" + ) + + +@pytest.mark.unit +def test_cli_run_update_item_metadata_concurrency_conflict(runner: CliRunner) -> None: + """Check run update-item-metadata exits 2 with a clear message on ConcurrencyConflictError.""" + from aignostics.system import ConcurrencyConflictError + + with patch("aignostics.application._cli.Service") as mock_service_cls: + mock_service_cls.return_value.application_run_update_item_custom_metadata.side_effect = ( + ConcurrencyConflictError("stale checksum") + ) + result = runner.invoke( + cli, + [ + "application", + "run", + "update-item-metadata", + "run-123", + "item-ext-id", + '{"key": "value"}', + "--checksum", + "old", + ], + ) + assert result.exit_code == 2 + assert "modified by another process" in result.output + + @pytest.mark.e2e @pytest.mark.timeout(timeout=180) @pytest.mark.sequential From c3da43565c19451b1679044a2fe411fedd2ff9e4 Mon Sep 17 00:00:00 2001 From: Andreas Kunft Date: Thu, 7 May 2026 14:19:10 +0200 Subject: [PATCH 3/6] fix: resolve SonarCloud S1192 duplicate string literal warnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract repeated string literals into named constants: - APPLICATION_CLI_SERVICE_PATCH_TARGET for the service patch path (5 uses) - _TEST_METADATA_JSON for the metadata stub (4 uses) - cast("dict[str, Any]") → cast(dict[str, Any]) in runs.py (4 uses) --- tests/aignostics/application/cli_test.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/aignostics/application/cli_test.py b/tests/aignostics/application/cli_test.py index 6dbba0fa2..308519558 100644 --- a/tests/aignostics/application/cli_test.py +++ b/tests/aignostics/application/cli_test.py @@ -53,6 +53,8 @@ DOCUMENT_MODEL_CARD_PDF = "model_card.pdf" DOCUMENT_MISSING_PDF = "missing.pdf" APPLICATION_CLI_CLIENT_PATCH_TARGET = "aignostics.application._cli.Client" +APPLICATION_CLI_SERVICE_PATCH_TARGET = "aignostics.application._cli.Service" +_TEST_METADATA_JSON = '{"key": "value"}' # Stub values reused across the document CLI tests. DOCUMENT_TEST_FAILURE_MESSAGE = "kaboom" # canonical exception body for unexpected-failure paths @@ -949,7 +951,7 @@ def test_cli_run_describe_json_includes_items(runner: CliRunner) -> None: with ( patch("aignostics.application._cli.PlatformService.get_user_info", return_value=mock_user_info), - patch("aignostics.application._cli.Service") as mock_service_cls, + patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_service_cls, ): mock_service_cls.return_value.application_run.return_value = mock_run_handle @@ -1214,7 +1216,7 @@ def test_cli_run_update_item_metadata_not_dict(runner: CliRunner) -> None: @pytest.mark.unit def test_cli_run_update_metadata_success_with_checksum(runner: CliRunner) -> None: """Check run update-metadata command succeeds and forwards --checksum to the service.""" - with patch("aignostics.application._cli.Service") as mock_service_cls: + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_service_cls: result = runner.invoke( cli, [ @@ -1222,7 +1224,7 @@ def test_cli_run_update_metadata_success_with_checksum(runner: CliRunner) -> Non "run", "update-metadata", "run-123", - '{"key": "value"}', + _TEST_METADATA_JSON, "--checksum", "abc123", ], @@ -1239,13 +1241,13 @@ def test_cli_run_update_metadata_concurrency_conflict(runner: CliRunner) -> None """Check run update-metadata exits 2 with a clear message on ConcurrencyConflictError.""" from aignostics.system import ConcurrencyConflictError - with patch("aignostics.application._cli.Service") as mock_service_cls: + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_service_cls: mock_service_cls.return_value.application_run_update_custom_metadata.side_effect = ConcurrencyConflictError( "stale checksum" ) result = runner.invoke( cli, - ["application", "run", "update-metadata", "run-123", '{"key": "value"}', "--checksum", "old"], + ["application", "run", "update-metadata", "run-123", _TEST_METADATA_JSON, "--checksum", "old"], ) assert result.exit_code == 2 assert "modified by another process" in result.output @@ -1254,7 +1256,7 @@ def test_cli_run_update_metadata_concurrency_conflict(runner: CliRunner) -> None @pytest.mark.unit def test_cli_run_update_item_metadata_success_with_checksum(runner: CliRunner) -> None: """Check run update-item-metadata command succeeds and forwards --checksum to the service.""" - with patch("aignostics.application._cli.Service") as mock_service_cls: + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_service_cls: result = runner.invoke( cli, [ @@ -1263,7 +1265,7 @@ def test_cli_run_update_item_metadata_success_with_checksum(runner: CliRunner) - "update-item-metadata", "run-123", "item-ext-id", - '{"key": "value"}', + _TEST_METADATA_JSON, "--checksum", "abc123", ], @@ -1280,7 +1282,7 @@ def test_cli_run_update_item_metadata_concurrency_conflict(runner: CliRunner) -> """Check run update-item-metadata exits 2 with a clear message on ConcurrencyConflictError.""" from aignostics.system import ConcurrencyConflictError - with patch("aignostics.application._cli.Service") as mock_service_cls: + with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_service_cls: mock_service_cls.return_value.application_run_update_item_custom_metadata.side_effect = ( ConcurrencyConflictError("stale checksum") ) @@ -1292,7 +1294,7 @@ def test_cli_run_update_item_metadata_concurrency_conflict(runner: CliRunner) -> "update-item-metadata", "run-123", "item-ext-id", - '{"key": "value"}', + _TEST_METADATA_JSON, "--checksum", "old", ], From 7e2318fca07eeff1a8e0c18cefa37bbb8ff08f5b Mon Sep 17 00:00:00 2001 From: Andreas Kunft Date: Fri, 8 May 2026 14:30:41 +0200 Subject: [PATCH 4/6] refactor(test): use public API imports in service_test.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace internal module imports with public API paths: - aignostics.platform.resources.runs.ApiException → aignostics.platform - aignostics.system._exceptions.ConcurrencyConflictError → aignostics.system --- tests/aignostics/application/service_test.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/aignostics/application/service_test.py b/tests/aignostics/application/service_test.py index e2234cbd0..f82cfa3d3 100644 --- a/tests/aignostics/application/service_test.py +++ b/tests/aignostics/application/service_test.py @@ -8,9 +8,8 @@ from typer.testing import CliRunner from aignostics.application import Service as ApplicationService -from aignostics.platform import NotFoundException, RunData, RunOutput -from aignostics.platform.resources.runs import ApiException -from aignostics.system._exceptions import ConcurrencyConflictError +from aignostics.platform import ApiException, NotFoundException, RunData, RunOutput +from aignostics.system import ConcurrencyConflictError from tests.constants_test import ( HETA_APPLICATION_ID, HETA_APPLICATION_VERSION, From 22a26a64d64f7e07d6ff162d480293e09460a118 Mon Sep 17 00:00:00 2001 From: Andreas Kunft Date: Fri, 8 May 2026 16:54:01 +0200 Subject: [PATCH 5/6] fix: resolve SonarCloud S1192 duplicate string literal in runs.py --- src/aignostics/platform/resources/runs.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/aignostics/platform/resources/runs.py b/src/aignostics/platform/resources/runs.py index a95623adb..b52a3f606 100644 --- a/src/aignostics/platform/resources/runs.py +++ b/src/aignostics/platform/resources/runs.py @@ -621,7 +621,7 @@ def update_custom_metadata( self._api.put_run_custom_metadata_v1_runs_run_id_custom_metadata_put( self.run_id, custom_metadata_update_request=CustomMetadataUpdateRequest( - custom_metadata=cast("dict[str, Any]", convert_to_json_serializable(custom_metadata)), + custom_metadata=cast(dict[str, Any], convert_to_json_serializable(custom_metadata)), # noqa: TC006 custom_metadata_checksum=custom_metadata_checksum, ), _request_timeout=settings().run_submit_timeout, @@ -660,7 +660,7 @@ def update_item_custom_metadata( self.run_id, external_id, custom_metadata_update_request=CustomMetadataUpdateRequest( - custom_metadata=cast("dict[str, Any]", convert_to_json_serializable(custom_metadata)), + custom_metadata=cast(dict[str, Any], convert_to_json_serializable(custom_metadata)), # noqa: TC006 custom_metadata_checksum=custom_metadata_checksum, ), _request_timeout=settings().run_submit_timeout, @@ -761,7 +761,7 @@ def submit( payload = RunCreationRequest( application_id=application_id, version_number=application_version, - custom_metadata=cast("dict[str, Any]", convert_to_json_serializable(custom_metadata)), + custom_metadata=cast(dict[str, Any], convert_to_json_serializable(custom_metadata)), # noqa: TC006 items=items, scheduling=scheduling, ) @@ -940,7 +940,7 @@ def _amend_input_items_with_sdk_metadata(items: builtins.list[ItemCreationReques validate_item_sdk_metadata(base_sdk_metadata) item_custom_metadata["sdk"] = base_sdk_metadata - item.custom_metadata = cast("dict[str, Any]", convert_to_json_serializable(item_custom_metadata)) + item.custom_metadata = cast(dict[str, Any], convert_to_json_serializable(item_custom_metadata)) # noqa: TC006 def _validate_input_items(self, payload: RunCreationRequest) -> None: """Validates the input items in a run creation request. From 2e8c224fce74bb34e9a8e04e0d3e87bcaf9edc5d Mon Sep 17 00:00:00 2001 From: Andreas Kunft Date: Fri, 8 May 2026 23:54:34 +0200 Subject: [PATCH 6/6] refactor: move ConcurrencyConflictError to platform, fix cast style, exit code 3, GUI handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move ConcurrencyConflictError from system/_exceptions to platform/_exceptions and re-export from aignostics.platform — eliminates application→system bidirectional dependency - Revert cast() string-form regression in runs.py (restore TC006-compliant cast("dict[str, Any]", ...) in all four call sites) - CLI conflict handlers now exit 3 (distinct from exit 2 for not-found) - Add specific ConcurrencyConflictError handler in GUI metadata editor with "reload and retry" guidance instead of generic error message - Update all imports and test assertions accordingly --- src/aignostics/application/_cli.py | 8 ++++---- .../_gui/_page_application_run_describe.py | 15 ++++++++++++++- src/aignostics/application/_service.py | 2 +- src/aignostics/platform/__init__.py | 2 ++ src/aignostics/platform/_exceptions.py | 10 ++++++++++ src/aignostics/platform/resources/runs.py | 8 ++++---- src/aignostics/system/__init__.py | 2 -- src/aignostics/system/_exceptions.py | 9 --------- tests/aignostics/application/cli_test.py | 12 ++++++------ tests/aignostics/application/service_test.py | 3 +-- 10 files changed, 42 insertions(+), 29 deletions(-) create mode 100644 src/aignostics/platform/_exceptions.py diff --git a/src/aignostics/application/_cli.py b/src/aignostics/application/_cli.py index 5b0b463a8..84e8a5a06 100644 --- a/src/aignostics/application/_cli.py +++ b/src/aignostics/application/_cli.py @@ -1227,7 +1227,7 @@ def run_update_metadata( """Update custom metadata for a run.""" import json # noqa: PLC0415 - from aignostics.system import ConcurrencyConflictError # noqa: PLC0415 + from aignostics.platform import ConcurrencyConflictError # noqa: PLC0415 logger.trace("Updating custom metadata for run with ID '{}'", run_id) @@ -1252,7 +1252,7 @@ def run_update_metadata( except ConcurrencyConflictError as e: logger.warning(f"Concurrency conflict updating metadata for run '{run_id}': {e}") console.print(f"[warning]Warning:[/warning] Metadata was modified by another process. Re-read and retry: {e}") - sys.exit(2) + sys.exit(3) except ValueError as e: logger.warning(f"Run ID '{run_id}' invalid or metadata invalid: {e}") console.print(f"[warning]Warning:[/warning] Run ID '{run_id}' invalid or metadata invalid: {e}") @@ -1284,7 +1284,7 @@ def run_update_item_metadata( """Update custom metadata for an item in a run.""" import json # noqa: PLC0415 - from aignostics.system import ConcurrencyConflictError # noqa: PLC0415 + from aignostics.platform import ConcurrencyConflictError # noqa: PLC0415 logger.trace("Updating custom metadata for item '{}' in run with ID '{}'", external_id, run_id) @@ -1311,7 +1311,7 @@ def run_update_item_metadata( except ConcurrencyConflictError as e: logger.warning("Concurrency conflict updating metadata for item '{}' in run '{}': {}", external_id, run_id, e) console.print(f"[warning]Warning:[/warning] Metadata was modified by another process. Re-read and retry: {e}") - sys.exit(2) + sys.exit(3) except ValueError as e: logger.warning( "Run ID '{}' or item external ID '{}' invalid or metadata invalid: {}", diff --git a/src/aignostics/application/_gui/_page_application_run_describe.py b/src/aignostics/application/_gui/_page_application_run_describe.py index 926aa767d..fae8726f6 100644 --- a/src/aignostics/application/_gui/_page_application_run_describe.py +++ b/src/aignostics/application/_gui/_page_application_run_describe.py @@ -19,7 +19,15 @@ ) from nicegui import run as nicegui_run -from aignostics.platform import ArtifactOutput, ItemOutput, ItemResult, ItemState, Run, RunState +from aignostics.platform import ( + ArtifactOutput, + ConcurrencyConflictError, + ItemOutput, + ItemResult, + ItemState, + Run, + RunState, +) from aignostics.third_party.showinfm.showinfm import show_in_file_manager from aignostics.utils import GUILocalFilePicker, get_user_data_directory @@ -747,6 +755,11 @@ async def handle_metadata_change(e: Any) -> None: # noqa: ANN401 ) ui.notify("Custom metadata updated successfully!", type="positive") ui.navigate.reload() + except ConcurrencyConflictError: + ui.notify( + "Metadata was modified by another process — reload the page and retry.", + type="warning", + ) except Exception as ex: ui.notify(f"Failed to update custom metadata: {ex!s}", type="negative") diff --git a/src/aignostics/application/_service.py b/src/aignostics/application/_service.py index dbfc1b679..0a2423a25 100644 --- a/src/aignostics/application/_service.py +++ b/src/aignostics/application/_service.py @@ -23,6 +23,7 @@ ApplicationSummary, ApplicationVersion, Client, + ConcurrencyConflictError, ForbiddenException, InputArtifact, InputItem, @@ -33,7 +34,6 @@ RunState, ) from aignostics.platform import Service as PlatformService -from aignostics.system import ConcurrencyConflictError from aignostics.utils import BaseService, Health, sanitize_path_component from aignostics.wsi import Service as WSIService diff --git a/src/aignostics/platform/__init__.py b/src/aignostics/platform/__init__.py index fea6f14f4..5de50f2e9 100644 --- a/src/aignostics/platform/__init__.py +++ b/src/aignostics/platform/__init__.py @@ -81,6 +81,7 @@ TOKEN_URL_STAGING, TOKEN_URL_TEST, ) +from ._exceptions import ConcurrencyConflictError from ._messages import AUTHENTICATION_FAILED, NOT_YET_IMPLEMENTED, UNKNOWN_ENDPOINT_URL from ._sdk_metadata import ( PipelineConfig, @@ -155,6 +156,7 @@ "Artifact", "ArtifactOutput", "Client", + "ConcurrencyConflictError", "Documents", "ForbiddenException", "InputArtifact", diff --git a/src/aignostics/platform/_exceptions.py b/src/aignostics/platform/_exceptions.py new file mode 100644 index 000000000..abab5bbff --- /dev/null +++ b/src/aignostics/platform/_exceptions.py @@ -0,0 +1,10 @@ +"""Exceptions of platform module.""" + + +class ConcurrencyConflictError(ValueError): + """Raised when an optimistic concurrency precondition (HTTP 412) fails. + + Subclasses ValueError so existing ``except ValueError`` callers still catch it, + while callers that need to distinguish a conflict from a bad-ID error can use + ``except ConcurrencyConflictError``. + """ diff --git a/src/aignostics/platform/resources/runs.py b/src/aignostics/platform/resources/runs.py index b52a3f606..a95623adb 100644 --- a/src/aignostics/platform/resources/runs.py +++ b/src/aignostics/platform/resources/runs.py @@ -621,7 +621,7 @@ def update_custom_metadata( self._api.put_run_custom_metadata_v1_runs_run_id_custom_metadata_put( self.run_id, custom_metadata_update_request=CustomMetadataUpdateRequest( - custom_metadata=cast(dict[str, Any], convert_to_json_serializable(custom_metadata)), # noqa: TC006 + custom_metadata=cast("dict[str, Any]", convert_to_json_serializable(custom_metadata)), custom_metadata_checksum=custom_metadata_checksum, ), _request_timeout=settings().run_submit_timeout, @@ -660,7 +660,7 @@ def update_item_custom_metadata( self.run_id, external_id, custom_metadata_update_request=CustomMetadataUpdateRequest( - custom_metadata=cast(dict[str, Any], convert_to_json_serializable(custom_metadata)), # noqa: TC006 + custom_metadata=cast("dict[str, Any]", convert_to_json_serializable(custom_metadata)), custom_metadata_checksum=custom_metadata_checksum, ), _request_timeout=settings().run_submit_timeout, @@ -761,7 +761,7 @@ def submit( payload = RunCreationRequest( application_id=application_id, version_number=application_version, - custom_metadata=cast(dict[str, Any], convert_to_json_serializable(custom_metadata)), # noqa: TC006 + custom_metadata=cast("dict[str, Any]", convert_to_json_serializable(custom_metadata)), items=items, scheduling=scheduling, ) @@ -940,7 +940,7 @@ def _amend_input_items_with_sdk_metadata(items: builtins.list[ItemCreationReques validate_item_sdk_metadata(base_sdk_metadata) item_custom_metadata["sdk"] = base_sdk_metadata - item.custom_metadata = cast(dict[str, Any], convert_to_json_serializable(item_custom_metadata)) # noqa: TC006 + item.custom_metadata = cast("dict[str, Any]", convert_to_json_serializable(item_custom_metadata)) def _validate_input_items(self, payload: RunCreationRequest) -> None: """Validates the input items in a run creation request. diff --git a/src/aignostics/system/__init__.py b/src/aignostics/system/__init__.py index 0397c147e..1dab2d033 100644 --- a/src/aignostics/system/__init__.py +++ b/src/aignostics/system/__init__.py @@ -1,12 +1,10 @@ """System module.""" from ._cli import cli -from ._exceptions import ConcurrencyConflictError from ._service import Service from ._settings import Settings __all__ = [ - "ConcurrencyConflictError", "Service", "Settings", "cli", diff --git a/src/aignostics/system/_exceptions.py b/src/aignostics/system/_exceptions.py index 9e878c921..e0a200463 100644 --- a/src/aignostics/system/_exceptions.py +++ b/src/aignostics/system/_exceptions.py @@ -7,12 +7,3 @@ class OpenAPISchemaError(ValueError): def __init__(self, error: Exception) -> None: """Initialize exception with the underlying error.""" super().__init__(f"Failed to load OpenAPI schema: {error}") - - -class ConcurrencyConflictError(ValueError): - """Raised when an optimistic concurrency precondition (HTTP 412) fails. - - Subclasses ValueError so existing ``except ValueError`` callers still catch it, - while callers that need to distinguish a conflict from a bad-ID error can use - ``except ConcurrencyConflictError``. - """ diff --git a/tests/aignostics/application/cli_test.py b/tests/aignostics/application/cli_test.py index 308519558..8544536ee 100644 --- a/tests/aignostics/application/cli_test.py +++ b/tests/aignostics/application/cli_test.py @@ -1238,8 +1238,8 @@ def test_cli_run_update_metadata_success_with_checksum(runner: CliRunner) -> Non @pytest.mark.unit def test_cli_run_update_metadata_concurrency_conflict(runner: CliRunner) -> None: - """Check run update-metadata exits 2 with a clear message on ConcurrencyConflictError.""" - from aignostics.system import ConcurrencyConflictError + """Check run update-metadata exits 3 with a clear message on ConcurrencyConflictError.""" + from aignostics.platform import ConcurrencyConflictError with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_service_cls: mock_service_cls.return_value.application_run_update_custom_metadata.side_effect = ConcurrencyConflictError( @@ -1249,7 +1249,7 @@ def test_cli_run_update_metadata_concurrency_conflict(runner: CliRunner) -> None cli, ["application", "run", "update-metadata", "run-123", _TEST_METADATA_JSON, "--checksum", "old"], ) - assert result.exit_code == 2 + assert result.exit_code == 3 assert "modified by another process" in result.output @@ -1279,8 +1279,8 @@ def test_cli_run_update_item_metadata_success_with_checksum(runner: CliRunner) - @pytest.mark.unit def test_cli_run_update_item_metadata_concurrency_conflict(runner: CliRunner) -> None: - """Check run update-item-metadata exits 2 with a clear message on ConcurrencyConflictError.""" - from aignostics.system import ConcurrencyConflictError + """Check run update-item-metadata exits 3 with a clear message on ConcurrencyConflictError.""" + from aignostics.platform import ConcurrencyConflictError with patch(APPLICATION_CLI_SERVICE_PATCH_TARGET) as mock_service_cls: mock_service_cls.return_value.application_run_update_item_custom_metadata.side_effect = ( @@ -1299,7 +1299,7 @@ def test_cli_run_update_item_metadata_concurrency_conflict(runner: CliRunner) -> "old", ], ) - assert result.exit_code == 2 + assert result.exit_code == 3 assert "modified by another process" in result.output diff --git a/tests/aignostics/application/service_test.py b/tests/aignostics/application/service_test.py index f82cfa3d3..90bf87c13 100644 --- a/tests/aignostics/application/service_test.py +++ b/tests/aignostics/application/service_test.py @@ -8,8 +8,7 @@ from typer.testing import CliRunner from aignostics.application import Service as ApplicationService -from aignostics.platform import ApiException, NotFoundException, RunData, RunOutput -from aignostics.system import ConcurrencyConflictError +from aignostics.platform import ApiException, ConcurrencyConflictError, NotFoundException, RunData, RunOutput from tests.constants_test import ( HETA_APPLICATION_ID, HETA_APPLICATION_VERSION,