diff --git a/server/osa/domain/deposition/command/create_convention.py b/server/osa/domain/deposition/command/create_convention.py index 4f690846..1e32fd3f 100644 --- a/server/osa/domain/deposition/command/create_convention.py +++ b/server/osa/domain/deposition/command/create_convention.py @@ -25,7 +25,12 @@ OciLimits, TableFeatureSpec, ) -from osa.domain.shared.model.source import IngesterDefinition +from osa.domain.shared.model.source import ( + IngesterDefinition, + IngesterLimits, + IngesterScheduleConfig, + InitialRunConfig, +) from osa.domain.shared.model.srn import ConventionSlug, SchemaId, SchemaIdentifier @@ -40,48 +45,74 @@ class DeployConventionSchema(BaseModel): class DeployConventionRelease(BaseModel): - """A hook's release block (== POST /hooks/{name}/releases body). + """A component's built release — a *pure build artifact*. - ``extra="forbid"`` + a required ``config`` make a client/server payload-shape - mismatch fail loudly at deploy (422, naming the offending field) rather than - being silently swallowed into an empty config that only fails at container - runtime. ``limits`` keeps its defaults — omitting resource limits is a valid, - explicit choice; a *misnamed* limits field is still caught by ``extra``. + ``config``/``limits`` are authored definition and live on the component, not + here (a `limits` tweak is a definition change, not a rebuild). ``extra="forbid"`` + makes a payload-shape mismatch fail loudly at deploy (422, naming the field). """ model_config = ConfigDict(extra="forbid") image: str digest: str - # Opaque, image-defined JSON object forwarded verbatim to the container — - # OSA never reads its keys. Required (don't default a dropped config to {}). - config: dict[str, Any] - limits: OciLimits = Field(default_factory=OciLimits) source_ref: str # REQUIRED — reproducibility anchor (FR-005) class DeployConventionHook(BaseModel): - """One hook in the bundled deploy: identity (name + fixed feature) + release.""" + """One hook in the bundled deploy: identity (name + fixed feature), authored + runtime (``config``/``limits``), and its built ``release``.""" model_config = ConfigDict(extra="forbid") name: HookName feature: TableFeatureSpec + # Opaque, image-defined JSON forwarded verbatim to the container — OSA never + # reads its keys. Required (don't default a dropped config to {}). + config: dict[str, Any] + limits: OciLimits = Field(default_factory=OciLimits) release: DeployConventionRelease def to_deploy(self) -> HookDeploy: + # Re-gather authored config/limits (on the hook) with the built image + # (in the release) into the internal runtime — internals are unchanged. return HookDeploy( identity=HookIdentity(name=self.name, feature=self.feature), runtime=OciConfig( image=self.release.image, digest=self.release.digest, - config=self.release.config, - limits=self.release.limits, + config=self.config, + limits=self.limits, ), source_ref=self.release.source_ref, ) +class DeployConventionIngester(BaseModel): + """The ingester in the bundled deploy — symmetric with a hook: authored + ``config``/``limits``/schedule + its built ``release``.""" + + model_config = ConfigDict(extra="forbid") + + name: str # build-fan-out key (cloud); not persisted server-side + config: dict[str, Any] | None = None + limits: IngesterLimits = Field(default_factory=IngesterLimits) + schedule: IngesterScheduleConfig | None = None + initial_run: InitialRunConfig | None = None + release: DeployConventionRelease + + def to_definition(self) -> IngesterDefinition: + return IngesterDefinition( + image=self.release.image, + digest=self.release.digest, + config=self.config, + limits=self.limits, + schedule=self.schedule, + initial_run=self.initial_run, + source_ref=self.release.source_ref, + ) + + class ExamplePayload(BaseModel): """Edge mirror of the ``Example`` VO — a worked example, rendered verbatim. @@ -156,7 +187,7 @@ class DeployConvention(Command): file_requirements: FileRequirements schema_block: DeployConventionSchema = Field(alias="schema") hooks: list[DeployConventionHook] = [] - ingester: IngesterDefinition | None = None + ingester: DeployConventionIngester | None = None # Author semantics — required; documentation is mandatory (#151, FR-015). docs: ConventionDocsPayload @@ -205,7 +236,7 @@ async def run(self, cmd: DeployConvention) -> ConventionCreated: schema_version=cmd.schema_block.version, schema_fields=cmd.schema_block.fields, hooks=[h.to_deploy() for h in cmd.hooks], - ingester=cmd.ingester, + ingester=cmd.ingester.to_definition() if cmd.ingester else None, docs=cmd.docs.to_vo(), built_by=built_by, ) diff --git a/server/osa/domain/shared/model/source.py b/server/osa/domain/shared/model/source.py index f2ee6055..49f73e59 100644 --- a/server/osa/domain/shared/model/source.py +++ b/server/osa/domain/shared/model/source.py @@ -91,3 +91,6 @@ class IngesterDefinition(ValueObject): limits: IngesterLimits = Field(default_factory=IngesterLimits) schedule: IngesterScheduleConfig | None = None initial_run: InitialRunConfig | None = None + # Reproducibility anchor for the build that produced ``image`` (parity with + # a hook's release ``source_ref``). ``None`` for ingesters predating the field. + source_ref: str | None = None diff --git a/server/tests/unit/domain/deposition/test_deploy_convention_dto.py b/server/tests/unit/domain/deposition/test_deploy_convention_dto.py new file mode 100644 index 00000000..f3346fc8 --- /dev/null +++ b/server/tests/unit/domain/deposition/test_deploy_convention_dto.py @@ -0,0 +1,177 @@ +"""The convention wire contract: config/limits are authored (on the component), +`release` is a pure build artifact, and hooks + ingesters are symmetric. + +These lock the edge DTOs (`DeployConventionHook`/`DeployConventionRelease`/ +`DeployConventionIngester`) and their `to_deploy`/`to_definition` re-gathering +into the *unchanged* internal `OciConfig`/`IngesterDefinition`. +""" + +from __future__ import annotations + +from typing import Any + +import pytest +from pydantic import ValidationError + +from osa.domain.deposition.command.create_convention import ( + DeployConvention, + DeployConventionHook, + DeployConventionIngester, + DeployConventionRelease, +) + + +def _hook(**overrides: Any) -> dict[str, Any]: + base: dict[str, Any] = { + "name": "detect", + "feature": { + "cardinality": "many", + "columns": [{"name": "score", "json_type": "number", "required": True}], + }, + "config": {"k": "v"}, + "limits": {"memory": "2g"}, + "release": {"image": "reg/x:1", "digest": "sha256:abc", "source_ref": "git:1"}, + } + base.update(overrides) + return base + + +def _ingester(**overrides: Any) -> dict[str, Any]: + base: dict[str, Any] = { + "name": "ingest", + "config": {"email": ""}, + "limits": {"memory": "1g"}, + "schedule": {"cron": "0 0 * * *"}, + "release": {"image": "reg/i:1", "digest": "sha256:def", "source_ref": "git:1"}, + } + base.update(overrides) + return base + + +def _body(**overrides: Any) -> dict[str, Any]: + base: dict[str, Any] = { + "title": "T", + "description": "d", + "file_requirements": { + "accepted_types": [".csv"], + "max_count": 5, + "max_file_size": 100, + }, + "schema": {"id": "s-id", "version": "1.0.0", "fields": []}, + "hooks": [_hook()], + "ingester": _ingester(), + "docs": { + "purpose": "p", + "example_questions": ["a?", "b?", "c?"], + "examples": [{"question": "a?", "query": "q", "interpretation": "i"}], + }, + } + base.update(overrides) + return base + + +# --- release is a pure build artifact --------------------------------------- + + +def test_release_is_pure_build_artifact() -> None: + rel = DeployConventionRelease.model_validate( + {"image": "reg/x:1", "digest": "sha256:abc", "source_ref": "git:1"} + ) + assert set(rel.model_dump()) == {"image", "digest", "source_ref"} + + +def test_release_rejects_config_and_limits() -> None: + # The old shape (config/limits inside release) is a loud 422 now. + for bad in ("config", "limits"): + with pytest.raises(ValidationError): + DeployConventionRelease.model_validate( + {"image": "i", "digest": "d", "source_ref": "g", bad: {}} + ) + + +# --- hook: authored config/limits on the component -------------------------- + + +def test_hook_carries_config_and_limits() -> None: + hook = DeployConventionHook.model_validate(_hook()) + assert hook.config == {"k": "v"} + assert hook.limits.memory == "2g" + assert set(hook.release.model_dump()) == {"image", "digest", "source_ref"} + + +def test_hook_config_is_required() -> None: + with pytest.raises(ValidationError): + DeployConventionHook.model_validate(_hook(config=None) | {"config": None}) + + +def test_hook_to_deploy_regathers_into_runtime() -> None: + hook = DeployConventionHook.model_validate(_hook()) + hd = hook.to_deploy() + # config/limits come from the hook, image/digest from the release. + assert hd.runtime.config == {"k": "v"} + assert hd.runtime.limits.memory == "2g" + assert hd.runtime.image == "reg/x:1" + assert hd.runtime.digest == "sha256:abc" + assert hd.source_ref == "git:1" + assert hd.identity.name.root == "detect" + + +# --- ingester: symmetric with hooks ----------------------------------------- + + +def test_ingester_is_symmetric_with_hooks() -> None: + ing = DeployConventionIngester.model_validate(_ingester()) + # authored config/limits/schedule on the component; build under release. + assert ing.config == {"email": ""} + assert ing.limits.memory == "1g" + assert ing.schedule.cron == "0 0 * * *" + assert set(ing.release.model_dump()) == {"image", "digest", "source_ref"} + + +def test_ingester_to_definition_regathers() -> None: + idef = DeployConventionIngester.model_validate(_ingester()).to_definition() + assert idef.image == "reg/i:1" + assert idef.digest == "sha256:def" + assert idef.config == {"email": ""} + assert idef.limits.memory == "1g" + assert idef.source_ref == "git:1" # provenance carried onto the ingester + + +def test_ingester_name_accepted_but_not_persisted() -> None: + # `name` is the cloud build-fan-out key; the internal ingester has none. + idef = DeployConventionIngester.model_validate(_ingester(name="anything")).to_definition() + assert "name" not in idef.model_dump() + + +def test_ingester_rejects_flat_image() -> None: + # No more flat image/digest — they live under release (extra=forbid). + with pytest.raises(ValidationError): + DeployConventionIngester.model_validate(_ingester(image="reg/i:1")) + + +# --- full body round-trip + optional omission ------------------------------- + + +def test_full_body_validates_and_maps() -> None: + cmd = DeployConvention.model_validate(_body()) + assert len(cmd.hooks) == 1 + assert cmd.hooks[0].to_deploy().runtime.config == {"k": "v"} + assert cmd.ingester is not None + assert cmd.ingester.to_definition().source_ref == "git:1" + + +def test_optional_fields_may_be_omitted() -> None: + # exclude_none producer output: omit ingester schedule/initial_run, hook + # limits (defaulted), docs optionals — the server accepts absence. + hook = _hook() + del hook["limits"] + ing = _ingester() + del ing["schedule"] + cmd = DeployConvention.model_validate(_body(hooks=[hook], ingester=ing)) + assert cmd.hooks[0].limits.memory == "1g" # OciLimits default + assert cmd.ingester is not None and cmd.ingester.schedule is None + + +def test_ingester_optional() -> None: + cmd = DeployConvention.model_validate(_body(ingester=None)) + assert cmd.ingester is None diff --git a/server/tests/unit/domain/deposition/test_deploy_convention_handler.py b/server/tests/unit/domain/deposition/test_deploy_convention_handler.py index ddc6cfb2..6d90001b 100644 --- a/server/tests/unit/domain/deposition/test_deploy_convention_handler.py +++ b/server/tests/unit/domain/deposition/test_deploy_convention_handler.py @@ -55,10 +55,10 @@ def _hook_payload(name: str) -> dict[str, Any]: "cardinality": "many", "columns": [{"name": "score", "json_type": "number", "required": True}], }, + "config": {}, "release": { "image": "ghcr.io/test/x", "digest": "sha256:abc", - "config": {}, "source_ref": "git:abc", }, } diff --git a/server/uv.lock b/server/uv.lock index a0b102af..a8cb0e1d 100644 --- a/server/uv.lock +++ b/server/uv.lock @@ -1188,7 +1188,7 @@ wheels = [ [[package]] name = "osa" -version = "0.0.5" +version = "0.0.6" source = { editable = "." } dependencies = [ { name = "aiodocker", marker = "(platform_machine == 'arm64' and sys_platform == 'darwin') or (platform_machine == 'aarch64' and sys_platform == 'linux') or (platform_machine == 'x86_64' and sys_platform == 'linux')" },