diff --git a/requirements/SWR-APPLICATION-2-17.md b/requirements/SWR-APPLICATION-2-17.md new file mode 100644 index 000000000..87aea6419 --- /dev/null +++ b/requirements/SWR-APPLICATION-2-17.md @@ -0,0 +1,10 @@ +--- +itemId: SWR-APPLICATION-2-17 +itemTitle: Update Run and Item Custom Metadata After Creation +itemHasParent: SHR-APPLICATION-2 +itemType: Requirement +Requirement type: FUNCTIONAL +Layer: System (backend logic) +--- + +System shall allow users to update the custom metadata of an existing application run, and of an individual item within a run, after creation with optional optimistic concurrency control. diff --git a/specifications/SPEC-APPLICATION-SERVICE.md b/specifications/SPEC-APPLICATION-SERVICE.md index 6dd42cf16..f21b58aa0 100644 --- a/specifications/SPEC-APPLICATION-SERVICE.md +++ b/specifications/SPEC-APPLICATION-SERVICE.md @@ -2,7 +2,7 @@ itemId: SPEC-APPLICATION-SERVICE itemTitle: Application Module Specification itemType: Software Item Spec -itemFulfills: SWR-APPLICATION-1-1, SWR-APPLICATION-1-2, SWR-APPLICATION-1-3, SWR-APPLICATION-2-3, SWR-APPLICATION-2-4, SHR-APPLICATION-3, SWR-APPLICATION-2-12, SWR-APPLICATION-2-11, SWR-APPLICATION-2-13, SWR-APPLICATION-2-14, SWR-APPLICATION-2-15, SWR-APPLICATION-2-16, SWR-APPLICATION-2-5, SWR-APPLICATION-2-7, SWR-APPLICATION-2-8, SWR-APPLICATION-2-9, SWR-APPLICATION-3-3 +itemFulfills: SWR-APPLICATION-1-1, SWR-APPLICATION-1-2, SWR-APPLICATION-1-3, SWR-APPLICATION-2-3, SWR-APPLICATION-2-4, SHR-APPLICATION-3, SWR-APPLICATION-2-12, SWR-APPLICATION-2-11, SWR-APPLICATION-2-13, SWR-APPLICATION-2-14, SWR-APPLICATION-2-15, SWR-APPLICATION-2-16, SWR-APPLICATION-2-17, SWR-APPLICATION-2-5, SWR-APPLICATION-2-7, SWR-APPLICATION-2-8, SWR-APPLICATION-2-9, SWR-APPLICATION-3-3 Module: Application Layer: Domain Service Version: 0.2.107 @@ -395,6 +395,59 @@ class Service: ForbiddenException: When the caller is not an admin of the requested org. """ pass + + def application_run_update_custom_metadata( + self, + run_id: str, + custom_metadata: dict[str, Any], + *, + custom_metadata_checksum: str | None = None, + enrich_sdk_metadata: bool = True, + ) -> None: + """Update the custom metadata of an existing run. + + Args: + run_id: Application run identifier. + custom_metadata: New custom metadata to attach to the run. + custom_metadata_checksum: Optional checksum for optimistic concurrency + control. When provided, a stale write is rejected by the platform + with HTTP 412 and surfaced as ConcurrencyConflictError. None skips + the precondition check. + enrich_sdk_metadata: When True (default), auto-generated SDK tracking + context is merged into the `sdk` field and schema-validated. When + False, custom_metadata is forwarded verbatim (sdk field neither + merged nor validated). + + Raises: + NotFoundException: When the run ID is not found. + ConcurrencyConflictError: When the checksum precondition fails (HTTP 412). + ValueError: When the run ID or metadata is invalid. + RuntimeError: When the update fails unexpectedly. + """ + pass + + 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, + enrich_sdk_metadata: bool = True, + ) -> None: + """Update the custom metadata of an item within a run. + + Same `custom_metadata_checksum` and `enrich_sdk_metadata` semantics as + `application_run_update_custom_metadata`, scoped to the item identified by + `external_id`. + + Raises: + NotFoundException: When the run or item is not found. + ConcurrencyConflictError: When the checksum precondition fails (HTTP 412). + ValueError: When the run ID or item external ID is invalid. + RuntimeError: When the update fails unexpectedly. + """ + pass ``` ### 4.2 CLI Interface @@ -422,6 +475,12 @@ uvx aignostics application [subcommand] [options] - `run cancel`: Cancel running application - `run result download`: Download run results - `run result delete`: Delete run results +- `run dump-metadata` / `run dump-item-metadata`: Dump a run's/item's custom metadata as JSON; + `--show-checksum` additionally emits the current `custom_metadata_checksum` +- `run update-metadata` / `run update-item-metadata`: Replace a run's/item's custom metadata. + `--checksum` guards the write with optimistic concurrency control (exit code 3 on conflict); + `--enrich-sdk-metadata / --no-enrich-sdk-metadata` (default enrich) controls whether the SDK + merges auto-generated tracking context into the `sdk` field or forwards it verbatim ### 4.3 GUI Interface @@ -500,6 +559,7 @@ Configuration is managed through environment variables with the prefix `AIGNOSTI | `FileNotFoundError` | Missing input files | File validation before upload | File path verification help | | `ApiException` | Platform API failures | Retry mechanism with recovery | API error details and guidance | | `ForbiddenException` | Caller not authorized for the requested org | Caught in CLI; exit 2 with access-denied message | User informed they lack permission | +| `ConcurrencyConflictError` | Custom-metadata update rejected (HTTP 412): metadata modified since the checksum was read | `ValueError` subclass; caught in CLI, exit 3 | User told to re-read and retry the update | ### 7.2 Input Validation diff --git a/specifications/SPEC_PLATFORM_SERVICE.md b/specifications/SPEC_PLATFORM_SERVICE.md index 7d6bc8493..1fe3b7a0a 100644 --- a/specifications/SPEC_PLATFORM_SERVICE.md +++ b/specifications/SPEC_PLATFORM_SERVICE.md @@ -2,7 +2,7 @@ itemId: SPEC-PLATFORM-SERVICE itemTitle: Platform Module Specification itemType: Software Item Spec -itemFulfills: SWR-APPLICATION-1-1, SWR-APPLICATION-1-2, SWR-APPLICATION-1-3, SWR-APPLICATION-2-1, SWR-APPLICATION-2-5, SWR-APPLICATION-2-6, SWR-APPLICATION-2-7, SWR-APPLICATION-2-9, SWR-APPLICATION-2-14, SWR-APPLICATION-2-15, SWR-APPLICATION-2-16, SWR-APPLICATION-3-1, SWR-APPLICATION-3-2, SWR-APPLICATION-3-3, SWR-APPLICATION-4-1, SWR-APPLICATION-4-2 +itemFulfills: SWR-APPLICATION-1-1, SWR-APPLICATION-1-2, SWR-APPLICATION-1-3, SWR-APPLICATION-2-1, SWR-APPLICATION-2-5, SWR-APPLICATION-2-6, SWR-APPLICATION-2-7, SWR-APPLICATION-2-9, SWR-APPLICATION-2-14, SWR-APPLICATION-2-15, SWR-APPLICATION-2-16, SWR-APPLICATION-2-17, SWR-APPLICATION-3-1, SWR-APPLICATION-3-2, SWR-APPLICATION-3-3, SWR-APPLICATION-4-1, SWR-APPLICATION-4-2 Module: Platform Layer: Platform Service Version: 1.2.0 @@ -544,6 +544,45 @@ class ApplicationRun(_AuthenticatedResource): ) -> None: """Ensures all AVAILABLE artifacts for an item are downloaded with checksum verification.""" + def update_custom_metadata( + self, + custom_metadata: dict[str, Any], + *, + custom_metadata_checksum: str | None = None, + enrich_sdk_metadata: bool = True, + ) -> None: + """Replace the run's custom metadata. + + Args: + custom_metadata: New custom metadata to attach to the run. + custom_metadata_checksum: Optional checksum for optimistic concurrency + control (read from ``ApplicationRunData.custom_metadata_checksum``). + When provided, the platform rejects the write with HTTP 412 if the + stored metadata was modified since the checksum was read. ``None`` + skips the precondition check. + enrich_sdk_metadata: When True (default), auto-generated SDK tracking + context is merged into ``custom_metadata["sdk"]`` and validated + against the SDK metadata schema. When False, ``custom_metadata`` is + forwarded verbatim — the ``sdk`` field is neither merged nor + validated — so a previously read ``sdk`` field round-trips unchanged. + """ + + def update_item_custom_metadata( + self, + external_id: str, + custom_metadata: dict[str, Any], + *, + custom_metadata_checksum: str | None = None, + enrich_sdk_metadata: bool = True, + ) -> None: + """Replace an item's custom metadata. + + Same ``custom_metadata_checksum`` (checksum read from + ``ItemResultReadResponse.custom_metadata_checksum``) and + ``enrich_sdk_metadata`` semantics as ``update_custom_metadata``, scoped to + the item identified by ``external_id``. + """ + def grant_access( self, subject_type: SubjectType, diff --git a/src/aignostics/application/CLAUDE.md b/src/aignostics/application/CLAUDE.md index 5a10ac61d..3c214d76b 100644 --- a/src/aignostics/application/CLAUDE.md +++ b/src/aignostics/application/CLAUDE.md @@ -794,6 +794,38 @@ Every run submitted through the application module automatically includes SDK me See `platform/CLAUDE.md` for detailed SDK metadata documentation and schema. +**Custom Metadata Updates: Checksum Concurrency Control & `enrich_sdk_metadata` Toggle (NEW):** + +`Service.application_run_update_custom_metadata()` / `..._update_item_custom_metadata()` (and their +`_static` wrappers) accept two keyword-only parameters, threaded through to the platform layer: + +- `custom_metadata_checksum: str | None` — pass the checksum from a prior read + (`RunData.custom_metadata_checksum` / `ItemResultReadResponse.custom_metadata_checksum`) for + optimistic concurrency control. If the metadata changed since the checksum was read, the platform + returns HTTP 412 and the service raises `aignostics.platform.ConcurrencyConflictError` (a + `ValueError` subclass) instead of silently overwriting a concurrent change. +- `enrich_sdk_metadata: bool = True` — set to `False` to forward `custom_metadata` (including any + `sdk` field) to the platform verbatim, skipping the automatic SDK metadata merge/validation. Use + this when round-tripping a previously dumped `sdk` field unchanged. + +CLI support (`_cli.py`): + +```bash +# Read-modify-write loop, safe under concurrent edits: +aignostics application run custom-metadata dump-metadata RUN_ID --show-checksum --pretty +# ... edit the dumped JSON ... +aignostics application run custom-metadata update-metadata RUN_ID "" \ + --checksum --no-enrich-sdk-metadata +``` + +- `dump-metadata` / `dump-item-metadata` gained `--show-checksum`, wrapping the output as + `{"custom_metadata": ..., "custom_metadata_checksum": ...}`. +- `update-metadata` / `update-item-metadata` gained `--checksum` and + `--enrich-sdk-metadata`/`--no-enrich-sdk-metadata`. A `ConcurrencyConflictError` from the service + exits the CLI with code 3 (distinct from the existing code 2 for not-found/invalid-ID errors). + +See `platform/CLAUDE.md` for the full checksum + `enrich_sdk_metadata` semantics. + **Signed URL Generation:** The `_download.py` module uses `platform.generate_signed_url()` to convert `gs://` URLs to time-limited signed URLs for downloads. diff --git a/src/aignostics/application/_cli.py b/src/aignostics/application/_cli.py index 15ecffdde..661ab7142 100644 --- a/src/aignostics/application/_cli.py +++ b/src/aignostics/application/_cli.py @@ -7,7 +7,7 @@ import zipfile from datetime import UTC, datetime from pathlib import Path -from typing import Annotated +from typing import Annotated, Any import typer from loguru import logger @@ -1008,6 +1008,16 @@ def run_describe( def run_dump_metadata( run_id: Annotated[str, typer.Argument(help="Id of the run to dump custom metadata for")], pretty: Annotated[bool, typer.Option(help="Pretty print JSON output with indentation")] = False, + show_checksum: Annotated[ + bool, + typer.Option( + "--show-checksum", + help=( + "Wrap the output as {'custom_metadata': ..., 'custom_metadata_checksum': ...} to obtain the " + "checksum required for optimistic concurrency control on a subsequent 'update-metadata --checksum'." + ), + ), + ] = False, ) -> None: """Dump custom metadata of a run as JSON to stdout.""" logger.trace("Dumping custom metadata for run with ID '{}'", run_id) @@ -1015,12 +1025,18 @@ def run_dump_metadata( try: run = Service().application_run(run_id).details() custom_metadata = run.custom_metadata if hasattr(run, "custom_metadata") else {} + output: dict[str, Any] | Any = custom_metadata + if show_checksum: + output = { + "custom_metadata": custom_metadata, + "custom_metadata_checksum": getattr(run, "custom_metadata_checksum", None), + } # Output JSON to stdout if pretty: - print(json.dumps(custom_metadata, indent=2)) + print(json.dumps(output, indent=2)) else: - print(json.dumps(custom_metadata)) + print(json.dumps(output)) logger.debug("Dumped custom metadata for run with ID '{}'", run_id) except NotFoundException: @@ -1038,6 +1054,17 @@ def run_dump_item_metadata( run_id: Annotated[str, typer.Argument(help="Id of the run containing the item")], external_id: Annotated[str, typer.Argument(help="External ID of the item to dump custom metadata for")], pretty: Annotated[bool, typer.Option(help="Pretty print JSON output with indentation")] = False, + show_checksum: Annotated[ + bool, + typer.Option( + "--show-checksum", + help=( + "Wrap the output as {'custom_metadata': ..., 'custom_metadata_checksum': ...} to obtain the " + "checksum required for optimistic concurrency control on a subsequent " + "'update-item-metadata --checksum'." + ), + ), + ] = False, ) -> None: """Dump custom metadata of an item as JSON to stdout.""" logger.trace("Dumping custom metadata for item '{}' in run with ID '{}'", external_id, run_id) @@ -1061,12 +1088,18 @@ def run_dump_item_metadata( sys.exit(2) custom_metadata = item.custom_metadata if hasattr(item, "custom_metadata") else {} + output: dict[str, Any] | Any = custom_metadata + if show_checksum: + output = { + "custom_metadata": custom_metadata, + "custom_metadata_checksum": getattr(item, "custom_metadata_checksum", None), + } # Output JSON to stdout if pretty: - print(json.dumps(custom_metadata, indent=2)) + print(json.dumps(output, indent=2)) else: - print(json.dumps(custom_metadata)) + print(json.dumps(output)) logger.debug("Dumped custom metadata for item '{}' in run with ID '{}'", external_id, run_id) except NotFoundException: @@ -1223,10 +1256,32 @@ 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 custom metadata checksum obtained from 'dump-metadata --show-checksum', used for " + "optimistic concurrency control. If the run's metadata was modified since the checksum was " + "read, the update is rejected (exit code 3)." + ), + ), + ] = None, + enrich_sdk_metadata: Annotated[ + bool, + typer.Option( + help=( + "Enrich the sdk field with auto-generated tracking context (submission, user, CI). " + "Use --no-enrich-sdk-metadata to forward the supplied sdk field unchanged." + ), + ), + ] = True, ) -> None: """Update custom metadata for a run.""" import json # noqa: PLC0415 + from aignostics.platform import ConcurrencyConflictError # noqa: PLC0415 + logger.trace("Updating custom metadata for run with ID '{}'", run_id) try: @@ -1240,9 +1295,18 @@ 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, + enrich_sdk_metadata=enrich_sdk_metadata, + ) 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 ConcurrencyConflictError: + logger.warning(f"Custom metadata for run with ID '{run_id}' was modified by another process.") + console.print("[warning]Warning:[/warning] Metadata was modified by another process. Re-read and retry.") + sys.exit(3) 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.") @@ -1264,10 +1328,32 @@ 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 custom metadata checksum obtained from 'dump-item-metadata --show-checksum', used " + "for optimistic concurrency control. If the item's metadata was modified since the checksum " + "was read, the update is rejected (exit code 3)." + ), + ), + ] = None, + enrich_sdk_metadata: Annotated[ + bool, + typer.Option( + help=( + "Enrich the sdk field with auto-generated tracking context (submission, user, CI). " + "Use --no-enrich-sdk-metadata to forward the supplied sdk field unchanged." + ), + ), + ] = True, ) -> None: """Update custom metadata for an item in a run.""" import json # noqa: PLC0415 + from aignostics.platform import ConcurrencyConflictError # noqa: PLC0415 + logger.trace("Updating custom metadata for item '{}' in run with ID '{}'", external_id, run_id) try: @@ -1281,9 +1367,23 @@ 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, + enrich_sdk_metadata=enrich_sdk_metadata, + ) 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 ConcurrencyConflictError: + logger.warning( + "Custom metadata for item '{}' in run with ID '{}' was modified by another process.", + external_id, + run_id, + ) + console.print("[warning]Warning:[/warning] Metadata was modified by another process. Re-read and retry.") + sys.exit(3) 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.") diff --git a/src/aignostics/application/_gui/_page_application_run_describe.py b/src/aignostics/application/_gui/_page_application_run_describe.py index cbbe29b61..120b1ca3f 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,9 +755,15 @@ async def handle_metadata_change(e: Any) -> None: # noqa: ANN401 Service.application_run_update_custom_metadata_static, run_id=run_id, custom_metadata=new_metadata, + custom_metadata_checksum=run_data.custom_metadata_checksum, ) 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 ba56bac40..89f7c4863 100644 --- a/src/aignostics/application/_service.py +++ b/src/aignostics/application/_service.py @@ -24,6 +24,7 @@ ApplicationSummary, ApplicationVersion, Client, + ConcurrencyConflictError, ForbiddenException, InputArtifact, InputItem, @@ -1126,21 +1127,37 @@ def application_run_update_custom_metadata( self, run_id: str, custom_metadata: dict[str, Any], + *, + custom_metadata_checksum: str | None = None, + enrich_sdk_metadata: bool = True, ) -> 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 obtained from a prior + read (e.g. ``RunData.custom_metadata_checksum``) used for optimistic + concurrency control. If the run's metadata was modified since the checksum + was read, the update is rejected with :class:`ConcurrencyConflictError`. + enrich_sdk_metadata (bool): If True (default), merge auto-generated SDK tracking + context into ``custom_metadata["sdk"]``. If False, forward ``custom_metadata`` + verbatim, preserving a caller-supplied ``sdk`` field unchanged. Raises: NotFoundException: If the application run with the given ID is not found. ValueError: If the run ID is invalid. + ConcurrencyConflictError: If the provided checksum no longer matches the run's + current custom metadata (optimistic concurrency conflict). 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, + enrich_sdk_metadata=enrich_sdk_metadata, + ) 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}" @@ -1151,6 +1168,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) @@ -1167,25 +1191,41 @@ 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, + enrich_sdk_metadata: bool = True, ) -> 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. See :meth:`application_run_update_custom_metadata`. + enrich_sdk_metadata (bool): See :meth:`application_run_update_custom_metadata`. Raises: NotFoundException: If the application run with the given ID is not found. ValueError: If the run ID is invalid. + ConcurrencyConflictError: If the provided checksum no longer matches the run's + current custom metadata (optimistic concurrency conflict). 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, + enrich_sdk_metadata=enrich_sdk_metadata, + ) 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, + enrich_sdk_metadata: bool = True, ) -> None: """Update custom metadata for an existing item in an application run. @@ -1193,10 +1233,19 @@ 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 obtained from a prior + read (e.g. ``ItemResultReadResponse.custom_metadata_checksum``) used for + optimistic concurrency control. If the item's metadata was modified since the + checksum was read, the update is rejected with :class:`ConcurrencyConflictError`. + enrich_sdk_metadata (bool): If True (default), merge auto-generated SDK tracking + context into ``custom_metadata["sdk"]``. If False, forward ``custom_metadata`` + verbatim, preserving a caller-supplied ``sdk`` field unchanged. Raises: NotFoundException: If the application run or item with the given IDs is not found. ValueError: If the run ID or item external ID is invalid. + ConcurrencyConflictError: If the provided checksum no longer matches the item's + current custom metadata (optimistic concurrency conflict). RuntimeError: If updating the item metadata fails unexpectedly. """ try: @@ -1208,6 +1257,8 @@ 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, + enrich_sdk_metadata=enrich_sdk_metadata, ) logger.trace( "Updated custom metadata for item '{}' in run with ID '{}'", @@ -1225,6 +1276,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 the " + f"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) @@ -1242,6 +1300,9 @@ 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, + enrich_sdk_metadata: bool = True, ) -> None: """Static wrapper for updating custom metadata for an item in an application run. @@ -1249,13 +1310,24 @@ 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. See :meth:`application_run_update_item_custom_metadata`. + enrich_sdk_metadata (bool): See :meth:`application_run_update_item_custom_metadata`. Raises: NotFoundException: If the application run or item with the given IDs is not found. ValueError: If the run ID or item external ID is invalid. + ConcurrencyConflictError: If the provided checksum no longer matches the item's + current custom metadata (optimistic concurrency conflict). 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, + enrich_sdk_metadata=enrich_sdk_metadata, + ) def application_run_cancel(self, run_id: str) -> None: """Cancel a run by its ID. diff --git a/src/aignostics/platform/CLAUDE.md b/src/aignostics/platform/CLAUDE.md index 5efb27307..a9cb819bd 100644 --- a/src/aignostics/platform/CLAUDE.md +++ b/src/aignostics/platform/CLAUDE.md @@ -613,6 +613,65 @@ def get_item_sdk_metadata_json_schema() -> dict[str, Any]: 11. **Item Metadata** (NEW) - Separate schema for item-level metadata with platform bucket information 12. **Metadata Updates** (NEW) - Update metadata via CLI (`aignostics application run custom-metadata update`) +### Optimistic Concurrency Control & `enrich_sdk_metadata` Toggle (NEW) + +**Checksum-based optimistic concurrency (`ConcurrencyConflictError`):** + +`RunData.custom_metadata_checksum` and `ItemResultReadResponse.custom_metadata_checksum` expose a +checksum for the run's/item's current custom metadata. `Run.update_custom_metadata()` and +`Run.update_item_custom_metadata()` accept a keyword-only `custom_metadata_checksum: str | None` +parameter that is forwarded verbatim on `CustomMetadataUpdateRequest`. If the checksum no longer +matches the server-side value (i.e. the metadata was modified since it was read), the platform +returns HTTP 412 Precondition Failed. The application service layer +(`Service.application_run_update_custom_metadata` / `..._update_item_custom_metadata`) maps this +412 to `aignostics.platform.ConcurrencyConflictError` — a `ValueError` subclass, so existing +`except ValueError` callers keep working, while callers that need to distinguish a stale-checksum +conflict from an invalid-ID error can catch `ConcurrencyConflictError` specifically. + +```python +from aignostics.platform import ConcurrencyConflictError, Client + +client = Client() +run = client.run("run-123") +details = run.details() + +try: + run.update_custom_metadata( + {**details.custom_metadata, "note": "reviewed"}, + custom_metadata_checksum=details.custom_metadata_checksum, + ) +except ConcurrencyConflictError: + # Metadata was modified since `details` was read — re-read and retry. + ... +``` + +**`enrich_sdk_metadata` toggle (preserve caller-supplied `sdk` field):** + +By default (`enrich_sdk_metadata=True`), `update_custom_metadata()` / `update_item_custom_metadata()` +merge auto-generated SDK tracking context into `custom_metadata["sdk"]` and validate it against the +SDK metadata schema, exactly as before. Passing `enrich_sdk_metadata=False` skips **both** the merge +and the schema validation — `custom_metadata` (including any `sdk` field the caller supplied) is +forwarded to the platform exactly as given. This lets a caller round-trip a previously dumped `sdk` +field (e.g. one containing `tags` or a `note` it wants to preserve unmodified) without the SDK +overwriting `submission`/`updated_at`/etc. on every write. + +**Combined read → modify → write loop (checksum + enrich toggle):** + +```bash +# 1. Dump current metadata together with its checksum +aignostics application run custom-metadata dump-metadata RUN_ID --show-checksum --pretty + +# 2. Edit the dumped JSON locally (e.g. add a tag, change a note) ... + +# 3. Write it back, guarding against concurrent modification and preserving `sdk` verbatim +aignostics application run update-metadata RUN_ID "$(cat edited.json)" \ + --checksum \ + --no-enrich-sdk-metadata +``` + +If another process modified the run's metadata between steps 1 and 3, step 3 exits with code 3 +(`ConcurrencyConflictError`) instead of silently overwriting the concurrent change. + **Testing:** Comprehensive test suite in `tests/aignostics/platform/sdk_metadata_test.py`: 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..ff3e230eb --- /dev/null +++ b/src/aignostics/platform/_exceptions.py @@ -0,0 +1,12 @@ +"""Exceptions of platform module.""" + +from __future__ import annotations + + +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 aa89ca896..e080a570d 100644 --- a/src/aignostics/platform/resources/runs.py +++ b/src/aignostics/platform/resources/runs.py @@ -604,26 +604,44 @@ def ensure_artifacts_downloaded( def update_custom_metadata( self, custom_metadata: dict[str, Any], + *, + custom_metadata_checksum: str | None = None, + enrich_sdk_metadata: bool = True, ) -> 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 rejects the update with HTTP 412 + (surfaced as :class:`~aignostics.platform.ConcurrencyConflictError` by the + application service layer) if the run's current custom metadata checksum does + not match, indicating the metadata was modified since the checksum was read + (e.g. via ``RunData.custom_metadata_checksum``). + enrich_sdk_metadata (bool): If True (default), merge auto-generated SDK tracking + context (submission, user, CI) into ``custom_metadata["sdk"]`` and validate it + against the SDK metadata schema. If False, ``custom_metadata`` is forwarded to + the platform verbatim: the ``sdk`` field (if any) is neither merged nor + validated. Use False together with the read-modify-write loop (dump + ``--show-checksum`` -> edit -> update ``--checksum ... --no-enrich-sdk-metadata``) + to preserve a caller-supplied ``sdk`` field unchanged. Raises: Exception: If the API request fails. """ custom_metadata = custom_metadata or {} - custom_metadata.setdefault("sdk", {}) - existing_sdk_metadata = custom_metadata.get("sdk", {}) - sdk_metadata = build_run_sdk_metadata(existing_sdk_metadata) - custom_metadata["sdk"].update(sdk_metadata) - validate_run_sdk_metadata(custom_metadata["sdk"]) + if enrich_sdk_metadata: + custom_metadata.setdefault("sdk", {}) + existing_sdk_metadata = custom_metadata.get("sdk", {}) + sdk_metadata = build_run_sdk_metadata(existing_sdk_metadata) + custom_metadata["sdk"].update(sdk_metadata) + validate_run_sdk_metadata(custom_metadata["sdk"]) 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()}, @@ -634,28 +652,46 @@ def update_item_custom_metadata( self, external_id: str, custom_metadata: dict[str, Any], + *, + custom_metadata_checksum: str | None = None, + enrich_sdk_metadata: bool = True, ) -> 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 rejects the update with HTTP 412 + (surfaced as :class:`~aignostics.platform.ConcurrencyConflictError` by the + application service layer) if the item's current custom metadata checksum does + not match, indicating the metadata was modified since the checksum was read + (e.g. via ``ItemResultReadResponse.custom_metadata_checksum``). + enrich_sdk_metadata (bool): If True (default), merge auto-generated SDK tracking + context into ``custom_metadata["sdk"]`` and validate it against the item SDK + metadata schema. If False, ``custom_metadata`` is forwarded to the platform + verbatim: the ``sdk`` field (if any) is neither merged nor validated. Use False + together with the read-modify-write loop (dump ``--show-checksum`` -> edit -> + update ``--checksum ... --no-enrich-sdk-metadata``) to preserve a + caller-supplied ``sdk`` field unchanged. Raises: Exception: If the API request fails. """ custom_metadata = custom_metadata or {} - custom_metadata.setdefault("sdk", {}) - existing_sdk_metadata = custom_metadata.get("sdk", {}) - sdk_metadata = build_item_sdk_metadata(existing_sdk_metadata) - custom_metadata["sdk"].update(sdk_metadata) - validate_item_sdk_metadata(custom_metadata["sdk"]) + if enrich_sdk_metadata: + custom_metadata.setdefault("sdk", {}) + existing_sdk_metadata = custom_metadata.get("sdk", {}) + sdk_metadata = build_item_sdk_metadata(existing_sdk_metadata) + custom_metadata["sdk"].update(sdk_metadata) + validate_item_sdk_metadata(custom_metadata["sdk"]) self._api.put_item_custom_metadata_by_run_v1_runs_run_id_items_external_id_custom_metadata_put( 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/tests/aignostics/application/TC-APPLICATION-CLI-07.feature b/tests/aignostics/application/TC-APPLICATION-CLI-07.feature new file mode 100644 index 000000000..29286f9ff --- /dev/null +++ b/tests/aignostics/application/TC-APPLICATION-CLI-07.feature @@ -0,0 +1,62 @@ +Feature: Update Run and Item Custom Metadata After Creation + + The system allows updating the custom metadata of an existing run or of an + individual item within a run after creation. Updates support optimistic + concurrency control via a metadata checksum, and allow the caller to choose + whether the SDK enriches the managed `sdk` metadata field with auto-generated + tracking context or forwards the supplied metadata unchanged. + + @tests:SPEC-APPLICATION-SERVICE + @tests:SPEC-PLATFORM-SERVICE + @tests:SWR-APPLICATION-2-17 + @id:TC-APPLICATION-CLI-07-01 + Scenario: System updates custom metadata of an existing run + Given the user has access to an existing application run + When the user updates the run's custom metadata with a valid metadata document + Then the system shall replace the run's custom metadata and confirm success + + @tests:SPEC-APPLICATION-SERVICE + @tests:SPEC-PLATFORM-SERVICE + @tests:SWR-APPLICATION-2-17 + @id:TC-APPLICATION-CLI-07-02 + Scenario: System rejects a run metadata update when the checksum is stale + Given the user has read a run's custom metadata together with its checksum + And the run's custom metadata was modified by another process afterwards + When the user updates the run's custom metadata supplying the previously read checksum + Then the system shall reject the update as a concurrency conflict and instruct the user to re-read and retry + + @tests:SPEC-APPLICATION-SERVICE + @tests:SPEC-PLATFORM-SERVICE + @tests:SWR-APPLICATION-2-17 + @id:TC-APPLICATION-CLI-07-03 + Scenario: System forwards the supplied sdk metadata unchanged when enrichment is disabled + Given the user has a custom metadata document containing a previously read sdk field + When the user updates the run's custom metadata with SDK metadata enrichment disabled + Then the system shall forward the custom metadata verbatim without merging or validating the sdk field + + @tests:SPEC-APPLICATION-SERVICE + @tests:SPEC-PLATFORM-SERVICE + @tests:SWR-APPLICATION-2-17 + @id:TC-APPLICATION-CLI-07-04 + Scenario: System enriches the sdk metadata field by default on update + Given the user has access to an existing application run + When the user updates the run's custom metadata without disabling enrichment + Then the system shall merge auto-generated SDK tracking context into the sdk field and validate it against the SDK metadata schema + + @tests:SPEC-APPLICATION-SERVICE + @tests:SPEC-PLATFORM-SERVICE + @tests:SWR-APPLICATION-2-17 + @id:TC-APPLICATION-CLI-07-05 + Scenario: System dumps a run's custom metadata together with its checksum + Given the user has access to an existing application run + When the user dumps the run's custom metadata requesting the checksum + Then the system shall emit the custom metadata together with its current checksum for use in a subsequent guarded update + + @tests:SPEC-APPLICATION-SERVICE + @tests:SPEC-PLATFORM-SERVICE + @tests:SWR-APPLICATION-2-17 + @id:TC-APPLICATION-CLI-07-06 + Scenario: System updates custom metadata of an item within a run + Given the user has access to an existing item within an application run + When the user updates the item's custom metadata with a valid metadata document + Then the system shall replace the item's custom metadata and confirm success diff --git a/tests/aignostics/application/cli_test.py b/tests/aignostics/application/cli_test.py index 95ea988bd..46b3c36c1 100644 --- a/tests/aignostics/application/cli_test.py +++ b/tests/aignostics/application/cli_test.py @@ -31,7 +31,7 @@ from aignostics.application import Service as ApplicationService from aignostics.cli import cli -from aignostics.platform import LIST_APPLICATION_RUNS_MAX_PAGE_SIZE +from aignostics.platform import LIST_APPLICATION_RUNS_MAX_PAGE_SIZE, ConcurrencyConflictError from aignostics.utils import Health, sanitize_path from tests.conftest import assert_parquet_geojson_parity, normalize_output, print_directory_structure from tests.constants_test import ( @@ -1160,6 +1160,198 @@ def test_cli_run_update_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_forwards_checksum(runner: CliRunner, record_property) -> None: + """Check run update-metadata forwards --checksum to the service as a keyword argument.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-01, SWR-APPLICATION-2-17, SPEC-APPLICATION-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 + mock_service_cls.return_value.application_run_update_custom_metadata.assert_called_once_with( + "run-123", {"key": "value"}, custom_metadata_checksum="abc123", enrich_sdk_metadata=True + ) + + +@pytest.mark.unit +def test_cli_run_update_metadata_default_enrich_sdk_metadata_true(runner: CliRunner, record_property) -> None: + """Check run update-metadata passes enrich_sdk_metadata=True to the service by default.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-04, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE") + with patch("aignostics.application._cli.Service") as mock_service_cls: + result = runner.invoke(cli, ["application", "run", "update-metadata", "run-123", '{"key": "value"}']) + + assert result.exit_code == 0 + mock_service_cls.return_value.application_run_update_custom_metadata.assert_called_once_with( + "run-123", {"key": "value"}, custom_metadata_checksum=None, enrich_sdk_metadata=True + ) + + +@pytest.mark.unit +def test_cli_run_update_metadata_no_enrich_sdk_metadata_reaches_service_false( + runner: CliRunner, record_property +) -> None: + """Check run update-metadata --no-enrich-sdk-metadata reaches the service as enrich_sdk_metadata=False.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-03, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE") + with patch("aignostics.application._cli.Service") as mock_service_cls: + result = runner.invoke( + cli, + [ + "application", + "run", + "update-metadata", + "run-123", + '{"key": "value"}', + "--no-enrich-sdk-metadata", + ], + ) + + assert result.exit_code == 0 + mock_service_cls.return_value.application_run_update_custom_metadata.assert_called_once_with( + "run-123", {"key": "value"}, custom_metadata_checksum=None, enrich_sdk_metadata=False + ) + + +@pytest.mark.unit +def test_cli_run_update_metadata_concurrency_conflict_exits_3(runner: CliRunner, record_property) -> None: + """Check run update-metadata exits with code 3 when the service raises ConcurrencyConflictError.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-02, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE") + 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"}']) + + assert result.exit_code == 3 + assert "modified by another process" in normalize_output(result.output) + + +@pytest.mark.unit +def test_cli_run_update_item_metadata_forwards_checksum(runner: CliRunner, record_property) -> None: + """Check run update-item-metadata forwards --checksum to the service as a keyword argument.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-06, SWR-APPLICATION-2-17, SPEC-APPLICATION-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 + 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", enrich_sdk_metadata=True + ) + + +@pytest.mark.unit +def test_cli_run_update_item_metadata_no_enrich_sdk_metadata_reaches_service_false( + runner: CliRunner, record_property +) -> None: + """Check run update-item-metadata --no-enrich-sdk-metadata reaches the service as enrich_sdk_metadata=False.""" + record_property( + "tested-item-id", + "TC-APPLICATION-CLI-07-03, TC-APPLICATION-CLI-07-06, SWR-APPLICATION-2-17, SPEC-APPLICATION-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"}', + "--no-enrich-sdk-metadata", + ], + ) + + assert result.exit_code == 0 + 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=None, enrich_sdk_metadata=False + ) + + +@pytest.mark.unit +def test_cli_run_update_item_metadata_concurrency_conflict_exits_3(runner: CliRunner, record_property) -> None: + """Check run update-item-metadata exits with code 3 when the service raises ConcurrencyConflictError.""" + record_property( + "tested-item-id", + "TC-APPLICATION-CLI-07-02, TC-APPLICATION-CLI-07-06, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE", + ) + 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"}'] + ) + + assert result.exit_code == 3 + assert "modified by another process" in normalize_output(result.output) + + +@pytest.mark.unit +def test_cli_run_dump_metadata_show_checksum(runner: CliRunner, record_property) -> None: + """Check run dump-metadata --show-checksum wraps output with the checksum.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-05, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE") + mock_run_data = MagicMock() + mock_run_data.custom_metadata = {"key": "value"} + mock_run_data.custom_metadata_checksum = "checksum-abc" + + mock_run_handle = MagicMock() + mock_run_handle.details.return_value = mock_run_data + + with patch("aignostics.application._cli.Service") as mock_service_cls: + mock_service_cls.return_value.application_run.return_value = mock_run_handle + result = runner.invoke(cli, ["application", "run", "dump-metadata", "run-123", "--show-checksum"]) + + assert result.exit_code == 0 + output = json.loads(result.stdout) + assert output == {"custom_metadata": {"key": "value"}, "custom_metadata_checksum": "checksum-abc"} + + +@pytest.mark.unit +def test_cli_run_dump_item_metadata_show_checksum(runner: CliRunner, record_property) -> None: + """Check run dump-item-metadata --show-checksum wraps output with the checksum.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-05, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE") + mock_item = MagicMock() + mock_item.external_id = "item-ext-id" + mock_item.custom_metadata = {"key": "value"} + mock_item.custom_metadata_checksum = "checksum-xyz" + + mock_run_handle = MagicMock() + mock_run_handle.results.return_value = iter([mock_item]) + + with patch("aignostics.application._cli.Service") as mock_service_cls: + mock_service_cls.return_value.application_run.return_value = mock_run_handle + result = runner.invoke( + cli, ["application", "run", "dump-item-metadata", "run-123", "item-ext-id", "--show-checksum"] + ) + + assert result.exit_code == 0 + output = json.loads(result.stdout) + assert output == {"custom_metadata": {"key": "value"}, "custom_metadata_checksum": "checksum-xyz"} + + @pytest.mark.integration def test_cli_run_execute_invalid_mapping_format(runner: CliRunner, tmp_path: Path) -> None: """Check execute command fails with invalid mapping format.""" diff --git a/tests/aignostics/application/service_test.py b/tests/aignostics/application/service_test.py index 297d692e8..4ab026072 100644 --- a/tests/aignostics/application/service_test.py +++ b/tests/aignostics/application/service_test.py @@ -1,14 +1,16 @@ """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 +from aignx.codegen.exceptions import ApiException from aignx.codegen.models import SubjectType from typer.testing import CliRunner from aignostics.application import Service as ApplicationService -from aignostics.platform import NotFoundException, RunData, RunOutput +from aignostics.platform import ConcurrencyConflictError, NotFoundException, RunData, RunOutput from tests.constants_test import ( HETA_APPLICATION_ID, HETA_APPLICATION_VERSION, @@ -512,7 +514,146 @@ 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, enrich_sdk_metadata=True + ) + + +@pytest.mark.unit +@patch("aignostics.application._service.Service._get_platform_client") +def test_application_run_update_custom_metadata_forwards_checksum(mock_get_client: MagicMock, record_property) -> None: + """Test that custom_metadata_checksum is forwarded to the platform layer.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-01, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE") + mock_client = MagicMock() + mock_run = MagicMock() + mock_client.run.return_value = mock_run + mock_get_client.return_value = mock_client + + service = ApplicationService() + custom_metadata = {"key": "value"} + + service.application_run_update_custom_metadata("run-123", custom_metadata, custom_metadata_checksum="abc123") + + mock_run.update_custom_metadata.assert_called_once_with( + custom_metadata, custom_metadata_checksum="abc123", enrich_sdk_metadata=True + ) + + +@pytest.mark.unit +@patch("aignostics.application._service.Service._get_platform_client") +def test_application_run_update_custom_metadata_forwards_enrich_sdk_metadata_false( + mock_get_client: MagicMock, record_property +) -> None: + """Test that enrich_sdk_metadata=False is forwarded to the platform layer.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-03, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE") + mock_client = MagicMock() + mock_run = MagicMock() + mock_client.run.return_value = mock_run + mock_get_client.return_value = mock_client + + service = ApplicationService() + custom_metadata = {"key": "value"} + + service.application_run_update_custom_metadata("run-123", custom_metadata, enrich_sdk_metadata=False) + + mock_run.update_custom_metadata.assert_called_once_with( + custom_metadata, custom_metadata_checksum=None, enrich_sdk_metadata=False + ) + + +@pytest.mark.unit +@patch("aignostics.application._service.Service._get_platform_client") +def test_application_run_update_custom_metadata_static_forwards_enrich_sdk_metadata( + mock_get_client: MagicMock, record_property +) -> None: + """Test that the static wrapper forwards enrich_sdk_metadata.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-03, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE") + mock_client = MagicMock() + mock_run = MagicMock() + mock_client.run.return_value = mock_run + mock_get_client.return_value = mock_client + + ApplicationService.application_run_update_custom_metadata_static( + "run-123", {"key": "value"}, enrich_sdk_metadata=False + ) + + mock_run.update_custom_metadata.assert_called_once_with( + {"key": "value"}, custom_metadata_checksum=None, enrich_sdk_metadata=False + ) + + +@pytest.mark.unit +@patch("aignostics.application._service.Service._get_platform_client") +def test_application_run_update_custom_metadata_static_forwards_checksum( + mock_get_client: MagicMock, record_property +) -> None: + """Test that the static wrapper forwards custom_metadata_checksum.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-01, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE") + mock_client = MagicMock() + mock_run = MagicMock() + mock_client.run.return_value = mock_run + mock_get_client.return_value = mock_client + + ApplicationService.application_run_update_custom_metadata_static( + "run-123", {"key": "value"}, custom_metadata_checksum="abc123" + ) + + mock_run.update_custom_metadata.assert_called_once_with( + {"key": "value"}, custom_metadata_checksum="abc123", enrich_sdk_metadata=True + ) + + +@pytest.mark.unit +@patch("aignostics.application._service.Service._get_platform_client") +def test_application_run_update_custom_metadata_concurrency_conflict( + mock_get_client: MagicMock, record_property +) -> None: + """Test that a 412 ApiException is mapped to ConcurrencyConflictError.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-02, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE") + mock_client = MagicMock() + mock_run = MagicMock() + mock_run.update_custom_metadata.side_effect = ApiException( + status=HTTPStatus.PRECONDITION_FAILED, reason="Precondition Failed" + ) + mock_client.run.return_value = mock_run + mock_get_client.return_value = mock_client + + service = ApplicationService() + + with pytest.raises(ConcurrencyConflictError): + service.application_run_update_custom_metadata("run-123", {"key": "value"}, custom_metadata_checksum="stale") + + # ConcurrencyConflictError is a ValueError subclass, so existing callers keep working. + with pytest.raises(ValueError): + 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_custom_metadata_non_412_api_exception_unchanged( + mock_get_client: MagicMock, record_property +) -> None: + """Test that a non-412 ApiException keeps the pre-existing behavior (RuntimeError).""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-02, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE") + mock_client = MagicMock() + mock_run = MagicMock() + mock_run.update_custom_metadata.side_effect = ApiException( + status=HTTPStatus.INTERNAL_SERVER_ERROR, reason="Internal Server Error" + ) + mock_client.run.return_value = mock_run + mock_get_client.return_value = mock_client + + service = ApplicationService() + + with pytest.raises(RuntimeError): + service.application_run_update_custom_metadata("run-123", {"key": "value"}) + + +@pytest.mark.unit +def test_concurrency_conflict_error_is_value_error(record_property) -> None: + """Test that ConcurrencyConflictError subclasses ValueError.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-02, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE") + assert issubclass(ConcurrencyConflictError, ValueError) @pytest.mark.unit @@ -549,7 +690,86 @@ 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, enrich_sdk_metadata=True + ) + + +@pytest.mark.unit +@patch("aignostics.application._service.Service._get_platform_client") +def test_application_run_update_item_custom_metadata_forwards_enrich_sdk_metadata_false( + mock_get_client: MagicMock, record_property +) -> None: + """Test that enrich_sdk_metadata=False is forwarded for item metadata updates.""" + record_property( + "tested-item-id", + "TC-APPLICATION-CLI-07-03, TC-APPLICATION-CLI-07-06, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE", + ) + mock_client = MagicMock() + mock_run = MagicMock() + mock_client.run.return_value = mock_run + mock_get_client.return_value = mock_client + + service = ApplicationService() + custom_metadata = {"key": "value"} + + service.application_run_update_item_custom_metadata( + "run-123", "item-ext-id", custom_metadata, enrich_sdk_metadata=False + ) + + mock_run.update_item_custom_metadata.assert_called_once_with( + "item-ext-id", custom_metadata, custom_metadata_checksum=None, enrich_sdk_metadata=False + ) + + +@pytest.mark.unit +@patch("aignostics.application._service.Service._get_platform_client") +def test_application_run_update_item_custom_metadata_static_forwards_enrich_sdk_metadata( + mock_get_client: MagicMock, record_property +) -> None: + """Test that the item static wrapper forwards enrich_sdk_metadata.""" + record_property( + "tested-item-id", + "TC-APPLICATION-CLI-07-03, TC-APPLICATION-CLI-07-06, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE", + ) + mock_client = MagicMock() + mock_run = MagicMock() + mock_client.run.return_value = mock_run + mock_get_client.return_value = mock_client + + ApplicationService.application_run_update_item_custom_metadata_static( + "run-123", "item-ext-id", {"key": "value"}, enrich_sdk_metadata=False + ) + + mock_run.update_item_custom_metadata.assert_called_once_with( + "item-ext-id", {"key": "value"}, custom_metadata_checksum=None, enrich_sdk_metadata=False + ) + + +@pytest.mark.unit +@patch("aignostics.application._service.Service._get_platform_client") +def test_application_run_update_item_custom_metadata_concurrency_conflict( + mock_get_client: MagicMock, record_property +) -> None: + """Test that a 412 ApiException for an item update is mapped to ConcurrencyConflictError.""" + record_property( + "tested-item-id", + "TC-APPLICATION-CLI-07-02, TC-APPLICATION-CLI-07-06, SWR-APPLICATION-2-17, SPEC-APPLICATION-SERVICE", + ) + mock_client = MagicMock() + mock_run = MagicMock() + mock_run.update_item_custom_metadata.side_effect = ApiException( + status=HTTPStatus.PRECONDITION_FAILED, reason="Precondition Failed" + ) + mock_client.run.return_value = mock_run + mock_get_client.return_value = mock_client + + service = ApplicationService() + + with pytest.raises(ConcurrencyConflictError): + service.application_run_update_item_custom_metadata( + "run-123", "item-ext-id", {"key": "value"}, custom_metadata_checksum="stale" + ) @pytest.mark.unit diff --git a/tests/aignostics/platform/resources/runs_test.py b/tests/aignostics/platform/resources/runs_test.py index 949bc74bf..3a0975834 100644 --- a/tests/aignostics/platform/resources/runs_test.py +++ b/tests/aignostics/platform/resources/runs_test.py @@ -1306,3 +1306,99 @@ def test_results_omits_none_filters(app_run, mock_api) -> None: assert "state" not in call_kwargs assert "termination_reason" not in call_kwargs assert "custom_metadata" not in call_kwargs + + +# ───────────────────────────────────────────────────────────────────────────── +# update_custom_metadata / update_item_custom_metadata: checksum + enrich_sdk_metadata +# ───────────────────────────────────────────────────────────────────────────── + +_PATCH_BUILD_RUN_SDK_METADATA = "aignostics.platform.resources.runs.build_run_sdk_metadata" +_PATCH_VALIDATE_RUN_SDK_METADATA = "aignostics.platform.resources.runs.validate_run_sdk_metadata" +_PATCH_BUILD_ITEM_SDK_METADATA = "aignostics.platform.resources.runs.build_item_sdk_metadata" +_PATCH_VALIDATE_ITEM_SDK_METADATA = "aignostics.platform.resources.runs.validate_item_sdk_metadata" + + +@pytest.mark.unit +def test_update_custom_metadata_forwards_checksum(app_run, mock_api, record_property) -> None: + """update_custom_metadata forwards custom_metadata_checksum on the update request.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-01, SWR-APPLICATION-2-17, SPEC-PLATFORM-SERVICE") + with patch(_PATCH_BUILD_RUN_SDK_METADATA, return_value={}), patch(_PATCH_VALIDATE_RUN_SDK_METADATA): + app_run.update_custom_metadata({"key": "value"}, custom_metadata_checksum="abc123") + + call_kwargs = mock_api.put_run_custom_metadata_v1_runs_run_id_custom_metadata_put.call_args[1] + request = call_kwargs["custom_metadata_update_request"] + assert request.custom_metadata_checksum == "abc123" + + +@pytest.mark.unit +def test_update_custom_metadata_enrich_default_calls_sdk_metadata_builders(app_run, mock_api, record_property) -> None: + """Default enrich_sdk_metadata=True merges and validates SDK metadata.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-04, SWR-APPLICATION-2-17, SPEC-PLATFORM-SERVICE") + mock_api.put_run_custom_metadata_v1_runs_run_id_custom_metadata_put.return_value = None + with ( + patch(_PATCH_BUILD_RUN_SDK_METADATA, return_value={"schema_version": "0.0.1"}) as mock_build, + patch(_PATCH_VALIDATE_RUN_SDK_METADATA) as mock_validate, + ): + app_run.update_custom_metadata({"key": "value"}) + + mock_build.assert_called_once() + mock_validate.assert_called_once() + call_kwargs = mock_api.put_run_custom_metadata_v1_runs_run_id_custom_metadata_put.call_args[1] + request = call_kwargs["custom_metadata_update_request"] + assert "sdk" in request.custom_metadata + assert request.custom_metadata["sdk"]["schema_version"] == "0.0.1" + + +@pytest.mark.unit +def test_update_custom_metadata_no_enrich_skips_sdk_metadata_builders(app_run, mock_api, record_property) -> None: + """enrich_sdk_metadata=False skips build/validate and forwards custom_metadata verbatim.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-03, SWR-APPLICATION-2-17, SPEC-PLATFORM-SERVICE") + with ( + patch(_PATCH_BUILD_RUN_SDK_METADATA) as mock_build, + patch(_PATCH_VALIDATE_RUN_SDK_METADATA) as mock_validate, + ): + app_run.update_custom_metadata({"key": "value"}, enrich_sdk_metadata=False) + + mock_build.assert_not_called() + mock_validate.assert_not_called() + call_kwargs = mock_api.put_run_custom_metadata_v1_runs_run_id_custom_metadata_put.call_args[1] + request = call_kwargs["custom_metadata_update_request"] + assert request.custom_metadata == {"key": "value"} + assert "sdk" not in request.custom_metadata + + +@pytest.mark.unit +def test_update_item_custom_metadata_forwards_checksum(app_run, mock_api, record_property) -> None: + """update_item_custom_metadata forwards custom_metadata_checksum on the update request.""" + record_property("tested-item-id", "TC-APPLICATION-CLI-07-06, SWR-APPLICATION-2-17, SPEC-PLATFORM-SERVICE") + with patch(_PATCH_BUILD_ITEM_SDK_METADATA, return_value={}), patch(_PATCH_VALIDATE_ITEM_SDK_METADATA): + app_run.update_item_custom_metadata("item-ext-id", {"key": "value"}, custom_metadata_checksum="xyz789") + + call_kwargs = ( + mock_api.put_item_custom_metadata_by_run_v1_runs_run_id_items_external_id_custom_metadata_put.call_args[1] + ) + request = call_kwargs["custom_metadata_update_request"] + assert request.custom_metadata_checksum == "xyz789" + + +@pytest.mark.unit +def test_update_item_custom_metadata_no_enrich_skips_sdk_metadata_builders(app_run, mock_api, record_property) -> None: + """enrich_sdk_metadata=False skips build/validate for items and forwards metadata verbatim.""" + record_property( + "tested-item-id", + "TC-APPLICATION-CLI-07-03, TC-APPLICATION-CLI-07-06, SWR-APPLICATION-2-17, SPEC-PLATFORM-SERVICE", + ) + with ( + patch(_PATCH_BUILD_ITEM_SDK_METADATA) as mock_build, + patch(_PATCH_VALIDATE_ITEM_SDK_METADATA) as mock_validate, + ): + app_run.update_item_custom_metadata("item-ext-id", {"key": "value"}, enrich_sdk_metadata=False) + + mock_build.assert_not_called() + mock_validate.assert_not_called() + call_kwargs = ( + mock_api.put_item_custom_metadata_by_run_v1_runs_run_id_items_external_id_custom_metadata_put.call_args[1] + ) + request = call_kwargs["custom_metadata_update_request"] + assert request.custom_metadata == {"key": "value"} + assert "sdk" not in request.custom_metadata