From 922e1211204d034511ca85d57f5758e8df4f327c Mon Sep 17 00:00:00 2001 From: Peter <101368063+ClassicMMT@users.noreply.github.com> Date: Fri, 17 Jul 2026 12:26:24 +1200 Subject: [PATCH] feat: add configurable discovery validation --- HISTORY.rst | 2 + datamasque/client/base.py | 29 +++++ .../client/discovery_config_libraries.py | 43 +++++++ datamasque/client/discovery_configs.py | 42 +++++++ datamasque/client/models/discovery_config.py | 13 ++- .../client/models/discovery_config_library.py | 13 ++- datamasque/client/models/status.py | 19 +++ tests/test_discovery_config_libraries.py | 53 +++++++++ tests/test_discovery_configs.py | 110 ++++++++++++++++++ 9 files changed, 320 insertions(+), 4 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 4cceae4..eb886bf 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -11,6 +11,8 @@ History * ``safe_data_preview`` on schema-discovery result columns and file-discovery locators, typed by ``kind``. * Added ``row_count`` to ``TableConstraints`` in schema-discovery ``table_metadata``. +* Added ``validate_discovery_config`` and ``validate_discovery_config_library``, plus a + ``validation_error_details`` field on the discovery config and library models. 1.1.7 (2026-07-14) ------------------ diff --git a/datamasque/client/base.py b/datamasque/client/base.py index 9aa3bea..ee99269 100644 --- a/datamasque/client/base.py +++ b/datamasque/client/base.py @@ -1,6 +1,7 @@ import logging import platform import sys +import time import warnings from contextlib import contextmanager from dataclasses import dataclass @@ -17,6 +18,7 @@ from datamasque.client.exceptions import ( DataMasqueApiError, + DataMasqueException, DataMasqueNotReadyError, DataMasqueTransportError, ) @@ -309,6 +311,33 @@ def _delete_if_exists(self, path: str, *, params: Optional[dict] = None) -> None self._raise_for_status(response) + def _poll_until_done( + self, + fetch: Callable[[], _T], + is_done: Callable[[_T], bool], + initial: _T, + *, + timeout: float, + poll_interval: float, + ) -> _T: + if is_done(initial): + return initial + + deadline = time.monotonic() + timeout + latest = initial + while time.monotonic() < deadline: + time.sleep(poll_interval) + latest = fetch() + if is_done(latest): + return latest + return latest + + def _delete_best_effort(self, delete: Callable[[], None], description: str) -> None: + try: + delete() + except DataMasqueException: + logger.warning('Failed to clean up %s; remove it manually.', description) + def _iter_paginated( self, path: str, diff --git a/datamasque/client/discovery_config_libraries.py b/datamasque/client/discovery_config_libraries.py index d657b3c..ab3f567 100644 --- a/datamasque/client/discovery_config_libraries.py +++ b/datamasque/client/discovery_config_libraries.py @@ -1,10 +1,12 @@ import logging +import uuid from typing import Optional from datamasque.client.base import BaseClient from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.discovery_config import DiscoveryConfigType from datamasque.client.models.discovery_config_library import DiscoveryConfigLibrary, DiscoveryConfigLibraryId +from datamasque.client.models.status import ValidationStatus logger = logging.getLogger(__name__) @@ -95,6 +97,7 @@ def create_discovery_config_library(self, library: DiscoveryConfigLibrary) -> Di library.id = created.id library.is_valid = created.is_valid library.validation_error = created.validation_error + library.validation_error_details = created.validation_error_details library.created = created.created library.modified = created.modified logger.info('Creation of discovery config library "%s" successful', library.name) @@ -123,6 +126,7 @@ def update_discovery_config_library(self, library: DiscoveryConfigLibrary) -> Di updated = DiscoveryConfigLibrary.model_validate(response.json()) library.is_valid = updated.is_valid library.validation_error = updated.validation_error + library.validation_error_details = updated.validation_error_details library.modified = updated.modified logger.debug('Update of discovery config library "%s" successful', library.name) return library @@ -141,6 +145,45 @@ def create_or_update_discovery_config_library(self, library: DiscoveryConfigLibr return self.create_discovery_config_library(library) + def validate_discovery_config_library( + self, + library: DiscoveryConfigLibrary, + *, + timeout: float = 60.0, + poll_interval: float = 1.0, + ) -> DiscoveryConfigLibrary: + """Validate a discovery config library's YAML server-side and return it with the verdict populated.""" + + temp = DiscoveryConfigLibrary( + name=f"__dm_validate_{uuid.uuid4().hex}", + namespace=library.namespace, + yaml=library.yaml, + config_type=library.config_type, + ) + created = self.create_discovery_config_library(temp) + settled = created + temp_id = created.id + if temp_id is not None: + library_id = temp_id + try: + settled = self._poll_until_done( + lambda: self.get_discovery_config_library(library_id), + lambda lib: lib.is_valid is not ValidationStatus.in_progress, + created, + timeout=timeout, + poll_interval=poll_interval, + ) + finally: + self._delete_best_effort( + lambda: self.delete_discovery_config_library_by_id_if_exists(library_id), + f"discovery config library `{library_id}`", + ) + + library.is_valid = settled.is_valid + library.validation_error = settled.validation_error + library.validation_error_details = settled.validation_error_details + return library + def delete_discovery_config_library_by_id_if_exists( self, library_id: DiscoveryConfigLibraryId, *, force: bool = False ) -> None: diff --git a/datamasque/client/discovery_configs.py b/datamasque/client/discovery_configs.py index 60f448d..e88dc95 100644 --- a/datamasque/client/discovery_configs.py +++ b/datamasque/client/discovery_configs.py @@ -1,10 +1,12 @@ import logging +import uuid from typing import Iterator, Optional from datamasque.client.base import BaseClient from datamasque.client.exceptions import DataMasqueApiError, DataMasqueException from datamasque.client.models.discovery_config import DiscoveryConfig, DiscoveryConfigId, DiscoveryConfigType from datamasque.client.models.pagination import Page +from datamasque.client.models.status import ValidationStatus logger = logging.getLogger(__name__) @@ -84,6 +86,7 @@ def create_discovery_config(self, config: DiscoveryConfig) -> DiscoveryConfig: config.id = created.id config.is_valid = created.is_valid config.validation_error = created.validation_error + config.validation_error_details = created.validation_error_details config.created = created.created config.modified = created.modified logger.info('Creation of discovery config "%s" successful', config.name) @@ -105,6 +108,7 @@ def update_discovery_config(self, config: DiscoveryConfig) -> DiscoveryConfig: updated = DiscoveryConfig.model_validate(response.json()) config.is_valid = updated.is_valid config.validation_error = updated.validation_error + config.validation_error_details = updated.validation_error_details config.modified = updated.modified logger.debug('Update of discovery config "%s" successful', config.name) return config @@ -123,6 +127,44 @@ def create_or_update_discovery_config(self, config: DiscoveryConfig) -> Discover return self.create_discovery_config(config) + def validate_discovery_config( + self, + config: DiscoveryConfig, + *, + timeout: float = 60.0, + poll_interval: float = 1.0, + ) -> DiscoveryConfig: + """Validate a discovery config's YAML server-side and return it with the verdict populated.""" + + temp = DiscoveryConfig( + name=f"__dm_validate_{uuid.uuid4().hex}", + yaml=config.yaml, + config_type=config.config_type, + ) + created = self.create_discovery_config(temp) + settled = created + temp_id = created.id + if temp_id is not None: + config_id = temp_id + try: + settled = self._poll_until_done( + lambda: self.get_discovery_config(config_id), + lambda c: c.is_valid is not ValidationStatus.in_progress, + created, + timeout=timeout, + poll_interval=poll_interval, + ) + finally: + self._delete_best_effort( + lambda: self.delete_discovery_config_by_id_if_exists(config_id), + f"discovery config `{config_id}`", + ) + + config.is_valid = settled.is_valid + config.validation_error = settled.validation_error + config.validation_error_details = settled.validation_error_details + return config + def delete_discovery_config_by_id_if_exists(self, config_id: DiscoveryConfigId) -> None: """ Deletes the discovery config with the given ID. diff --git a/datamasque/client/models/discovery_config.py b/datamasque/client/models/discovery_config.py index 09b123f..870152d 100644 --- a/datamasque/client/models/discovery_config.py +++ b/datamasque/client/models/discovery_config.py @@ -2,9 +2,9 @@ from datetime import datetime from typing import Any, NewType, Optional -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, ConfigDict, Field, model_validator -from datamasque.client.models.status import ValidationStatus +from datamasque.client.models.status import ValidationErrorDetails, ValidationStatus, promote_error_locations DiscoveryConfigId = NewType("DiscoveryConfigId", str) @@ -49,5 +49,14 @@ class DiscoveryConfig(BaseModel): """Validation status; may be `in_progress` briefly after creating a large config.""" validation_error: Optional[str] = Field(default=None, exclude=True) """Human-readable validation error, or `None` when valid.""" + validation_error_details: list[ValidationErrorDetails] = Field(default_factory=list, exclude=True) + """Structured, positional validation errors.""" created: Optional[datetime] = Field(default=None, exclude=True) modified: Optional[datetime] = Field(default=None, exclude=True) + + @model_validator(mode="before") + @classmethod + def _promote_error_locations(cls, data: object) -> object: + """Flatten the server's `errors` payload into `validation_error_details`.""" + + return promote_error_locations(data) diff --git a/datamasque/client/models/discovery_config_library.py b/datamasque/client/models/discovery_config_library.py index 095cf18..f0cdcb1 100644 --- a/datamasque/client/models/discovery_config_library.py +++ b/datamasque/client/models/discovery_config_library.py @@ -1,10 +1,10 @@ from datetime import datetime from typing import NewType, Optional -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, ConfigDict, Field, model_validator from datamasque.client.models.discovery_config import DiscoveryConfigType -from datamasque.client.models.status import ValidationStatus +from datamasque.client.models.status import ValidationErrorDetails, ValidationStatus, promote_error_locations DiscoveryConfigLibraryId = NewType("DiscoveryConfigLibraryId", str) @@ -29,5 +29,14 @@ class DiscoveryConfigLibrary(BaseModel): """Validation status; libraries are validated synchronously on create/update.""" validation_error: Optional[str] = Field(default=None, exclude=True) """Human-readable validation error, or `None` when valid.""" + validation_error_details: list[ValidationErrorDetails] = Field(default_factory=list, exclude=True) + """Structured, positional validation errors.""" created: Optional[datetime] = Field(default=None, exclude=True) modified: Optional[datetime] = Field(default=None, exclude=True) + + @model_validator(mode="before") + @classmethod + def _promote_error_locations(cls, data: object) -> object: + """Flatten a server `errors` payload into `validation_error_details`, if one is present.""" + + return promote_error_locations(data) diff --git a/datamasque/client/models/status.py b/datamasque/client/models/status.py index 0fd8935..086b549 100644 --- a/datamasque/client/models/status.py +++ b/datamasque/client/models/status.py @@ -31,6 +31,25 @@ class ValidationErrorDetails(BaseModel): column_number: Optional[int] = None +def promote_error_locations(data: object) -> object: + """Flatten a server `errors` payload into a `validation_error_details` list.""" + + if not isinstance(data, dict) or "errors" not in data: + return data + data = dict(data) + raw_errors = data.pop("errors") + entries: list[object] = [] + if isinstance(raw_errors, dict): + for value in raw_errors.values(): + if isinstance(value, list): + entries.extend(value) + elif isinstance(raw_errors, list): + entries = raw_errors + if entries: + data["validation_error_details"] = entries + return data + + class MaskingRunStatus(enum.Enum): """List of valid masking run statuses.""" diff --git a/tests/test_discovery_config_libraries.py b/tests/test_discovery_config_libraries.py index 5ad094b..cc75f63 100644 --- a/tests/test_discovery_config_libraries.py +++ b/tests/test_discovery_config_libraries.py @@ -623,3 +623,56 @@ def test_delete_discovery_config_library_by_name_not_found( assert m.call_count == 1 assert m.request_history[0].method == "GET" + + +def _build_library_response(is_valid: str, **extra: object) -> dict[str, object]: + return { + "id": LIBRARY_ID_1, + "name": "__dm_validate_stub", + "namespace": "", + "config_type": "database", + "is_valid": is_valid, + "validation_error": None, + "usage_count": 0, + **extra, + } + + +def test_validate_discovery_config_library_valid(client: DataMasqueClient) -> None: + library = DiscoveryConfigLibrary(name="mylib", namespace="org", yaml="blocks: {}\n", config_type="database") + with requests_mock.Mocker() as m: + m.post( + "http://test-server/api/discovery/config-libraries/", + json=_build_library_response("valid", namespace="org"), + status_code=201, + ) + m.delete(f"http://test-server/api/discovery/config-libraries/{LIBRARY_ID_1}/", status_code=204) + result = client.validate_discovery_config_library(library) + + assert result.is_valid is ValidationStatus.valid + assert result.validation_error is None + assert result.name == "mylib" + assert result.namespace == "org" + assert result.id is None + + post = m.request_history[0] + assert post.method == "POST" + assert post.json()["name"].startswith("__dm_validate_") + assert m.request_history[-1].method == "DELETE" + + +def test_validate_discovery_config_library_invalid(client: DataMasqueClient) -> None: + library = DiscoveryConfigLibrary(name="mylib", yaml="bad: [\n", config_type="file") + with requests_mock.Mocker() as m: + m.post( + "http://test-server/api/discovery/config-libraries/", + json=_build_library_response("invalid", config_type="file", validation_error="could not parse YAML"), + status_code=201, + ) + m.delete(f"http://test-server/api/discovery/config-libraries/{LIBRARY_ID_1}/", status_code=204) + result = client.validate_discovery_config_library(library) + + assert result.is_valid is ValidationStatus.invalid + assert result.validation_error == "could not parse YAML" + assert not any(r.method == "GET" for r in m.request_history) + assert m.request_history[-1].method == "DELETE" diff --git a/tests/test_discovery_configs.py b/tests/test_discovery_configs.py index caa4b31..f5cc79a 100644 --- a/tests/test_discovery_configs.py +++ b/tests/test_discovery_configs.py @@ -518,3 +518,113 @@ def test_unwrap_discovery_config_id_raises_without_id() -> None: config = DiscoveryConfig(name="x", config_type="database") with pytest.raises(ValueError, match="id is None"): unwrap_discovery_config_id(config) + + +def _build_config_response(is_valid: str, **extra: object) -> dict[str, object]: + return { + "id": CONFIG_ID_1, + "name": "__dm_validate_stub", + "config_type": "database", + "is_valid": is_valid, + "validation_error": None, + **extra, + } + + +def test_discovery_config_promotes_errors_payload() -> None: + config = DiscoveryConfig.model_validate( + _build_config_response( + "invalid", + validation_error="Unknown mask 'foo'", + errors={"config_yaml": [{"message": "Unknown mask 'foo'", "line_number": 3, "column_number": 5}]}, + ) + ) + + assert config.is_valid is ValidationStatus.invalid + assert len(config.validation_error_details) == 1 + detail = config.validation_error_details[0] + assert detail.message == "Unknown mask 'foo'" + assert detail.line_number == 3 + assert detail.column_number == 5 + assert "errors" not in (config.model_extra or {}) + + +def test_validate_discovery_config_valid_sync(client: DataMasqueClient, discovery_config: DiscoveryConfig) -> None: + with requests_mock.Mocker() as m: + m.post("http://test-server/api/discovery/configs/", json=_build_config_response("valid"), status_code=201) + m.delete(f"http://test-server/api/discovery/configs/{CONFIG_ID_1}/", status_code=204) + result = client.validate_discovery_config(discovery_config) + + assert result.is_valid is ValidationStatus.valid + assert result.validation_error is None + assert result.validation_error_details == [] + assert result.name == "test_config" + assert result.id is None + + post = m.request_history[0] + assert post.method == "POST" + body = post.json() + assert body["name"].startswith("__dm_validate_") + assert body["name"] != "test_config" + assert body["config_yaml"] == discovery_config.yaml + assert m.request_history[-1].method == "DELETE" + + +def test_validate_discovery_config_invalid_sync(client: DataMasqueClient, discovery_config: DiscoveryConfig) -> None: + response = _build_config_response( + "invalid", + validation_error="Unknown mask 'foo'", + errors={"config_yaml": [{"message": "Unknown mask 'foo'", "line_number": 3, "column_number": 5}]}, + ) + with requests_mock.Mocker() as m: + m.post("http://test-server/api/discovery/configs/", json=response, status_code=201) + m.delete(f"http://test-server/api/discovery/configs/{CONFIG_ID_1}/", status_code=204) + result = client.validate_discovery_config(discovery_config) + + assert result.is_valid is ValidationStatus.invalid + assert result.validation_error == "Unknown mask 'foo'" + assert len(result.validation_error_details) == 1 + assert result.validation_error_details[0].line_number == 3 + assert result.validation_error_details[0].column_number == 5 + + +def test_validate_discovery_config_async_settles(client: DataMasqueClient, discovery_config: DiscoveryConfig) -> None: + with requests_mock.Mocker() as m: + m.post("http://test-server/api/discovery/configs/", json=_build_config_response("in_progress"), status_code=201) + m.get( + f"http://test-server/api/discovery/configs/{CONFIG_ID_1}/", + [ + {"json": _build_config_response("in_progress"), "status_code": 200}, + {"json": _build_config_response("valid"), "status_code": 200}, + ], + ) + m.delete(f"http://test-server/api/discovery/configs/{CONFIG_ID_1}/", status_code=204) + result = client.validate_discovery_config(discovery_config, timeout=5.0, poll_interval=0.01) + + assert result.is_valid is ValidationStatus.valid + assert len([r for r in m.request_history if r.method == "GET"]) == 2 + assert m.request_history[-1].method == "DELETE" + + +def test_validate_discovery_config_timeout(client: DataMasqueClient, discovery_config: DiscoveryConfig) -> None: + with requests_mock.Mocker() as m: + m.post("http://test-server/api/discovery/configs/", json=_build_config_response("in_progress"), status_code=201) + m.get(f"http://test-server/api/discovery/configs/{CONFIG_ID_1}/", json=_build_config_response("in_progress")) + m.delete(f"http://test-server/api/discovery/configs/{CONFIG_ID_1}/", status_code=204) + result = client.validate_discovery_config(discovery_config, timeout=0.05, poll_interval=0.01) + + assert result.is_valid is ValidationStatus.in_progress + assert any(r.method == "DELETE" for r in m.request_history) + + +def test_validate_discovery_config_cleans_up_when_polling_errors( + client: DataMasqueClient, discovery_config: DiscoveryConfig +) -> None: + with requests_mock.Mocker() as m: + m.post("http://test-server/api/discovery/configs/", json=_build_config_response("in_progress"), status_code=201) + m.get(f"http://test-server/api/discovery/configs/{CONFIG_ID_1}/", status_code=500) + m.delete(f"http://test-server/api/discovery/configs/{CONFIG_ID_1}/", status_code=204) + with pytest.raises(DataMasqueApiError): + client.validate_discovery_config(discovery_config, timeout=5.0, poll_interval=0.01) + + assert any(r.method == "DELETE" for r in m.request_history)