From 59d92d9c8eeac2ebe409b0da455fd488df4eeb1c Mon Sep 17 00:00:00 2001 From: Peter <101368063+ClassicMMT@users.noreply.github.com> Date: Mon, 27 Jul 2026 14:25:45 +1200 Subject: [PATCH 1/3] feat: add discovery config and library validation - Add `validate_discovery_config` and `validate_discovery_config_library`. - Add `validation_error_details` to `DiscoveryConfig` and `usage_count` to `DiscoveryConfigLibrary`. - Reject configs and libraries with no YAML content, with an error that says how to fix it. - Ensure validating leaves nothing behind, even when it fails. - Let pydantic parse the nested errors payload instead of a custom validator - Add DataMasqueArgumentError --- HISTORY.rst | 10 + datamasque/client/base.py | 7 + .../client/discovery_config_libraries.py | 57 ++++- datamasque/client/discovery_configs.py | 57 ++++- datamasque/client/exceptions.py | 10 + datamasque/client/models/discovery_config.py | 13 +- .../client/models/discovery_config_library.py | 2 + tests/test_discovery_config_libraries.py | 157 +++++++++++-- tests/test_discovery_configs.py | 207 +++++++++++++++++- 9 files changed, 492 insertions(+), 28 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 983534b..349e5ef 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -2,6 +2,16 @@ History ======= +1.2.2 (unreleased) +------------------ + +* Added ``validation_error_details`` to ``DiscoveryConfig``. +* Added ``usage_count`` to ``DiscoveryConfigLibrary``. +* Added ``validate_discovery_config`` and ``validate_discovery_config_library``. +* ``update_discovery_config``, ``validate_discovery_config``, and ``validate_discovery_config_library`` +* Now raise ``DataMasqueArgumentError`` when the passed entity has no ``yaml`` content, + instead of sending a request the server rejects. + 1.2.1 (2026-07-30) ------------------ diff --git a/datamasque/client/base.py b/datamasque/client/base.py index 9aa3bea..3b045bb 100644 --- a/datamasque/client/base.py +++ b/datamasque/client/base.py @@ -17,6 +17,7 @@ from datamasque.client.exceptions import ( DataMasqueApiError, + DataMasqueException, DataMasqueNotReadyError, DataMasqueTransportError, ) @@ -309,6 +310,12 @@ def _delete_if_exists(self, path: str, *, params: Optional[dict] = None) -> None self._raise_for_status(response) + def _delete_best_effort(self, delete: Callable[[], None], description: str) -> None: + try: + delete() + except DataMasqueException as e: + logger.warning("Failed to clean up %s; remove it manually. Error: %s", description, e) + def _iter_paginated( self, path: str, diff --git a/datamasque/client/discovery_config_libraries.py b/datamasque/client/discovery_config_libraries.py index 25c69a3..a355ffb 100644 --- a/datamasque/client/discovery_config_libraries.py +++ b/datamasque/client/discovery_config_libraries.py @@ -1,8 +1,9 @@ import logging +import uuid from typing import Optional from datamasque.client.base import BaseClient -from datamasque.client.exceptions import DataMasqueApiError +from datamasque.client.exceptions import DataMasqueApiError, DataMasqueArgumentError from datamasque.client.models.discovery_config_library import DiscoveryConfigLibrary, DiscoveryConfigLibraryId logger = logging.getLogger(__name__) @@ -76,15 +77,21 @@ def create_discovery_config_library(self, library: DiscoveryConfigLibrary) -> Di Creates a new discovery config library on the server. Sets the library's server-assigned fields - (`id`, `is_valid`, `validation_error`, `created`, `modified`) and returns the library. + (`id`, `is_valid`, `validation_error`, `usage_count`, `created`, `modified`) and returns the library. """ + if not library.yaml: + raise DataMasqueArgumentError( + "Cannot create a discovery config library without YAML content (yaml is empty)" + ) + data = library.model_dump(exclude_none=True, by_alias=True, mode="json") response = self.make_request("POST", "/api/discovery/config-libraries/", data=data) created = DiscoveryConfigLibrary.model_validate(response.json()) library.id = created.id library.is_valid = created.is_valid library.validation_error = created.validation_error + library.usage_count = created.usage_count library.created = created.created library.modified = created.modified logger.info('Creation of discovery config library "%s" successful', library.name) @@ -99,11 +106,13 @@ def update_discovery_config_library(self, library: DiscoveryConfigLibrary) -> Di """ if library.id is None: - raise ValueError("Cannot update a discovery config library that has not been created yet (id is None)") + raise DataMasqueArgumentError( + "Cannot update a discovery config library that has not been created yet (id is None)" + ) - if library.yaml is None: - raise ValueError( - "Cannot update a discovery config library without YAML content (yaml is None); " + if not library.yaml: + raise DataMasqueArgumentError( + "Cannot update a discovery config library without YAML content (yaml is empty or unset); " "list results omit YAML, so fetch the full library with `get_discovery_config_library` first" ) @@ -112,6 +121,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.usage_count = updated.usage_count library.modified = updated.modified logger.debug('Update of discovery config library "%s" successful', library.name) return library @@ -130,6 +140,41 @@ 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) -> DiscoveryConfigLibrary: + """Validates a discovery config library against the server without persisting it.""" + + if not library.yaml: + raise DataMasqueArgumentError( + "Cannot validate a discovery config library without YAML content (yaml is empty or unset); " + "list results omit YAML, so fetch the full library with `get_discovery_config_library` first" + ) + + temporary = DiscoveryConfigLibrary( + name=f"dm_python_validate_{uuid.uuid4().hex}", + namespace=library.namespace, + yaml=library.yaml, + ) + data = temporary.model_dump(exclude_none=True, by_alias=True, mode="json") + response = self.make_request("POST", "/api/discovery/config-libraries/", data=data) + + payload = response.json() + raw_id = payload.get("id") if isinstance(payload, dict) else None + created_id = DiscoveryConfigLibraryId(raw_id) if isinstance(raw_id, str) else None + + try: + created = DiscoveryConfigLibrary.model_validate(payload) + library.is_valid = created.is_valid + library.validation_error = created.validation_error + finally: + if created_id is not None: + self._delete_best_effort( + lambda: self.delete_discovery_config_library_by_id_if_exists(created_id), + f'temporary validation library "{temporary.name}"', + ) + + logger.debug('Validation of discovery config library "%s" complete', library.name) + 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..7226e30 100644 --- a/datamasque/client/discovery_configs.py +++ b/datamasque/client/discovery_configs.py @@ -1,8 +1,9 @@ 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.exceptions import DataMasqueApiError, DataMasqueArgumentError, DataMasqueException from datamasque.client.models.discovery_config import DiscoveryConfig, DiscoveryConfigId, DiscoveryConfigType from datamasque.client.models.pagination import Page @@ -75,15 +76,20 @@ def create_discovery_config(self, config: DiscoveryConfig) -> DiscoveryConfig: Creates a new discovery config on the server. Sets the config's server-assigned fields - (`id`, `is_valid`, `validation_error`, `created`, `modified`) and returns the config. + (`id`, `is_valid`, `validation_error`, `validation_error_details`, `created`, `modified`) + and returns the config. """ + if not config.yaml: + raise DataMasqueArgumentError("Cannot create a discovery config without YAML content (yaml is empty)") + data = config.model_dump(exclude_none=True, by_alias=True, mode="json") response = self.make_request("POST", "/api/discovery/configs/", data=data) created = DiscoveryConfig.model_validate(response.json()) 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) @@ -94,17 +100,24 @@ def update_discovery_config(self, config: DiscoveryConfig) -> DiscoveryConfig: Performs a full update of the discovery config. The config must have its `id` set - (i.e., it must have been previously created or retrieved from the server). + and its `yaml` content present. """ if config.id is None: - raise ValueError("Cannot update a discovery config that has not been created yet (id is None)") + raise DataMasqueArgumentError("Cannot update a discovery config that has not been created yet (id is None)") + + if not config.yaml: + raise DataMasqueArgumentError( + "Cannot update a discovery config without YAML content (yaml is empty or unset); " + "list results omit YAML, so fetch the full config with `get_discovery_config` first" + ) data = config.model_dump(exclude_none=True, by_alias=True, mode="json") response = self.make_request("PUT", f"/api/discovery/configs/{config.id}/", data=data) 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 +136,42 @@ def create_or_update_discovery_config(self, config: DiscoveryConfig) -> Discover return self.create_discovery_config(config) + def validate_discovery_config(self, config: DiscoveryConfig) -> DiscoveryConfig: + """Validates a discovery config against the server.""" + + if not config.yaml: + raise DataMasqueArgumentError( + "Cannot validate a discovery config without YAML content (yaml is empty or unset); " + "list results omit YAML, so fetch the full config with `get_discovery_config` first" + ) + + temporary = DiscoveryConfig( + name=f"dm_python_validate_{uuid.uuid4().hex}", + yaml=config.yaml, + config_type=config.config_type, + ) + data = temporary.model_dump(exclude_none=True, by_alias=True, mode="json") + response = self.make_request("POST", "/api/discovery/configs/", data=data) + + payload = response.json() + raw_id = payload.get("id") if isinstance(payload, dict) else None + created_id = DiscoveryConfigId(raw_id) if isinstance(raw_id, str) else None + + try: + created = DiscoveryConfig.model_validate(payload) + config.is_valid = created.is_valid + config.validation_error = created.validation_error + config.validation_error_details = created.validation_error_details + finally: + if created_id is not None: + self._delete_best_effort( + lambda: self.delete_discovery_config_by_id_if_exists(created_id), + f'temporary validation config "{temporary.name}"', + ) + + logger.debug('Validation of discovery config "%s" complete', config.name) + 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/exceptions.py b/datamasque/client/exceptions.py index 9943b8b..d24cb59 100644 --- a/datamasque/client/exceptions.py +++ b/datamasque/client/exceptions.py @@ -9,6 +9,16 @@ class DataMasqueUserError(DataMasqueException): """Raised when error occurs during user creation or configuration.""" +class DataMasqueArgumentError(DataMasqueException): + """ + Raised when a client method is given an object it cannot act on. + + Covers arguments the client rejects without contacting the server, such as + updating a record that has no `id` yet, or sending a config whose `yaml` is + empty. + """ + + class DataMasqueApiError(DataMasqueException): """ Raised when the DataMasque server responds to a request with a non-2xx status code. diff --git a/datamasque/client/models/discovery_config.py b/datamasque/client/models/discovery_config.py index 09b123f..d1a0aa8 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 AliasChoices, AliasPath, BaseModel, ConfigDict, Field -from datamasque.client.models.status import ValidationStatus +from datamasque.client.models.status import ValidationErrorDetails, ValidationStatus 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.""" + # Deliberately not `validation_errors`: + # this is a different shape to `Ruleset.validation_errors`, + # and would sit one character from `validation_error` above. + validation_error_details: list[ValidationErrorDetails] = Field( + default_factory=list, + exclude=True, + validation_alias=AliasChoices(AliasPath("errors", "config_yaml"), "validation_error_details"), + ) + """Structured, positional validation errors.""" created: Optional[datetime] = Field(default=None, exclude=True) modified: Optional[datetime] = Field(default=None, exclude=True) diff --git a/datamasque/client/models/discovery_config_library.py b/datamasque/client/models/discovery_config_library.py index 9dbec49..d73b82f 100644 --- a/datamasque/client/models/discovery_config_library.py +++ b/datamasque/client/models/discovery_config_library.py @@ -27,5 +27,7 @@ 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.""" + usage_count: Optional[int] = Field(default=None, exclude=True) + """Number of active discovery configs that import this library.""" created: Optional[datetime] = Field(default=None, exclude=True) modified: Optional[datetime] = Field(default=None, exclude=True) diff --git a/tests/test_discovery_config_libraries.py b/tests/test_discovery_config_libraries.py index d72cff3..f733cef 100644 --- a/tests/test_discovery_config_libraries.py +++ b/tests/test_discovery_config_libraries.py @@ -1,17 +1,20 @@ """Tests for discovery config library support in the DataMasque client.""" +import logging from datetime import datetime -from typing import Any +from typing import Any, Optional import pytest +import requests import requests_mock +from pydantic import ValidationError from datamasque.client import ( DataMasqueClient, DiscoveryConfigLibrary, DiscoveryConfigLibraryId, ) -from datamasque.client.exceptions import DataMasqueApiError +from datamasque.client.exceptions import DataMasqueApiError, DataMasqueArgumentError from datamasque.client.models.status import ValidationStatus LIBRARY_ID_1 = "aaaaaaaa-1111-2222-3333-444444444444" @@ -93,10 +96,12 @@ def test_list_discovery_config_libraries( assert libraries[0].namespace == "org" assert libraries[0].yaml is None assert libraries[0].is_valid is ValidationStatus.valid + assert libraries[0].usage_count == 0 assert libraries[1].id == DiscoveryConfigLibraryId(LIBRARY_ID_2) assert libraries[1].name == "another_library" assert libraries[1].is_valid is ValidationStatus.invalid assert libraries[1].validation_error == "bad yaml" + assert libraries[1].usage_count == 2 def test_list_discovery_config_libraries_empty(client: DataMasqueClient) -> None: @@ -125,6 +130,7 @@ def test_get_discovery_config_library(client: DataMasqueClient, sample_library_d assert library.namespace == "org" assert library.yaml == "labels: []\nmetadata_rules: []\nidd_rules: []\n" assert library.is_valid is ValidationStatus.valid + assert library.usage_count == 0 def test_get_discovery_config_library_by_name_found( @@ -248,6 +254,7 @@ def test_create_discovery_config_library(client: DataMasqueClient, config_librar "config_yaml": "labels: []\nmetadata_rules: []\nidd_rules: []\n", "is_valid": "valid", "validation_error": None, + "usage_count": 0, "created": "2025-06-01T10:00:00Z", "modified": "2025-06-01T10:00:00Z", } @@ -263,16 +270,16 @@ def test_create_discovery_config_library(client: DataMasqueClient, config_librar assert result is config_library assert result.id == DiscoveryConfigLibraryId(LIBRARY_ID_1) assert result.is_valid is ValidationStatus.valid + assert result.usage_count == 0 assert result.created == datetime.fromisoformat("2025-06-01T10:00:00+00:00") assert result.modified == datetime.fromisoformat("2025-06-01T10:00:00+00:00") request_body = m.last_request.json() - assert request_body["name"] == "test_library" - assert request_body["namespace"] == "test_ns" - assert "config_type" not in request_body - assert request_body["config_yaml"] == "labels: []\nmetadata_rules: []\nidd_rules: []\n" - read_only_fields = {"id", "is_valid", "validation_error", "created", "modified"} - assert not read_only_fields & request_body.keys() + assert request_body == { + "name": "test_library", + "namespace": "test_ns", + "config_yaml": "labels: []\nmetadata_rules: []\nidd_rules: []\n", + } def test_create_discovery_config_library_reports_validation_error( @@ -312,6 +319,7 @@ def test_update_discovery_config_library(client: DataMasqueClient, config_librar "config_yaml": "labels: []\nmetadata_rules: []\nidd_rules: []\n", "is_valid": "valid", "validation_error": None, + "usage_count": 3, "created": "2025-06-01T10:00:00Z", "modified": "2025-06-02T10:00:00Z", } @@ -327,19 +335,21 @@ def test_update_discovery_config_library(client: DataMasqueClient, config_librar assert result is config_library assert result.is_valid is ValidationStatus.valid assert result.validation_error is None + assert result.usage_count == 3 assert result.modified == datetime.fromisoformat("2025-06-02T10:00:00+00:00") request_body = m.last_request.json() - assert request_body["name"] == "test_library" - assert request_body["config_yaml"] == "labels: []\nmetadata_rules: []\nidd_rules: []\n" - read_only_fields = {"id", "is_valid", "validation_error", "created", "modified"} - assert not read_only_fields & request_body.keys() + assert request_body == { + "name": "test_library", + "namespace": "test_ns", + "config_yaml": "labels: []\nmetadata_rules: []\nidd_rules: []\n", + } def test_update_discovery_config_library_no_id_raises( client: DataMasqueClient, config_library: DiscoveryConfigLibrary ) -> None: - with pytest.raises(ValueError, match="id is None"): + with pytest.raises(DataMasqueArgumentError, match="id is None"): client.update_discovery_config_library(config_library) @@ -349,7 +359,7 @@ def test_update_discovery_config_library_without_yaml_raises( config_library.id = DiscoveryConfigLibraryId(LIBRARY_ID_1) config_library.yaml = None - with pytest.raises(ValueError, match="yaml is None"): + with pytest.raises(DataMasqueArgumentError, match="without YAML content"): client.update_discovery_config_library(config_library) @@ -493,3 +503,122 @@ def test_delete_discovery_config_library_by_name_not_found( assert m.call_count == 1 assert m.request_history[0].method == "GET" + + +@pytest.mark.parametrize( + ("is_valid", "validation_error"), + [("valid", None), ("invalid", 'duplicate label "email"')], +) +def test_validate_discovery_config_library_round_trips_outcome( + client: DataMasqueClient, + config_library: DiscoveryConfigLibrary, + is_valid: str, + validation_error: Optional[str], +) -> None: + create_response = { + "id": LIBRARY_ID_1, + "name": "dm_python_validate_abc", + "namespace": "test_ns", + "is_valid": is_valid, + "validation_error": validation_error, + "usage_count": 0, + } + + with requests_mock.Mocker() as m: + m.post("http://test-server/api/discovery/config-libraries/", json=create_response, status_code=201) + delete = m.delete(f"http://test-server/api/discovery/config-libraries/{LIBRARY_ID_1}/", status_code=204) + result = client.validate_discovery_config_library(config_library) + + assert result is config_library + assert result.is_valid is ValidationStatus(is_valid) + assert result.validation_error == validation_error + assert result.name == "test_library" + assert result.namespace == "test_ns" + assert delete.called_once + + request_body = m.request_history[0].json() + assert request_body["name"].startswith("dm_python_validate_") + assert request_body["namespace"] == "test_ns" + assert "config_yaml" in request_body + + +def test_validate_discovery_config_library_without_yaml_raises( + client: DataMasqueClient, sample_library_list_response: list[dict[str, Any]] +) -> None: + + with requests_mock.Mocker() as m: + m.get("http://test-server/api/discovery/config-libraries/", json=sample_library_list_response, status_code=200) + library = client.list_discovery_config_libraries()[0] + assert library.yaml is None + + with pytest.raises(DataMasqueArgumentError, match="without YAML content"): + client.validate_discovery_config_library(library) + + assert not any(request.method == "POST" for request in m.request_history) + + +def test_create_discovery_config_library_with_empty_yaml_raises(client: DataMasqueClient) -> None: + library = DiscoveryConfigLibrary(name="test_library", yaml="") + + with requests_mock.Mocker() as m: + with pytest.raises(DataMasqueArgumentError, match="without YAML content"): + client.create_discovery_config_library(library) + + assert not any(request.method == "POST" for request in m.request_history) + + +def test_validate_discovery_config_library_with_empty_yaml_raises(client: DataMasqueClient) -> None: + library = DiscoveryConfigLibrary(name="test_library", yaml="") + + with requests_mock.Mocker() as m: + with pytest.raises(DataMasqueArgumentError, match="without YAML content"): + client.validate_discovery_config_library(library) + + assert not any(request.method == "POST" for request in m.request_history) + + +def test_validate_discovery_config_library_deletes_temp_library_when_response_is_rejected( + client: DataMasqueClient, config_library: DiscoveryConfigLibrary +) -> None: + rejected_response = { + "id": LIBRARY_ID_1, + "name": "dm_python_validate_abc", + "namespace": "", + "is_valid": "not_a_real_status", + "validation_error": None, + "usage_count": 0, + } + + with requests_mock.Mocker() as m: + m.post("http://test-server/api/discovery/config-libraries/", json=rejected_response, status_code=201) + delete = m.delete(f"http://test-server/api/discovery/config-libraries/{LIBRARY_ID_1}/", status_code=204) + + with pytest.raises(ValidationError): + client.validate_discovery_config_library(config_library) + + assert delete.called_once + + +def test_validate_discovery_config_library_returns_outcome_when_cleanup_cannot_reach_server( + client: DataMasqueClient, config_library: DiscoveryConfigLibrary, caplog: pytest.LogCaptureFixture +) -> None: + create_response = { + "id": LIBRARY_ID_1, + "name": "dm_python_validate_abc", + "namespace": "", + "is_valid": "valid", + "validation_error": None, + "usage_count": 0, + } + + with requests_mock.Mocker() as m: + m.post("http://test-server/api/discovery/config-libraries/", json=create_response, status_code=201) + m.delete( + f"http://test-server/api/discovery/config-libraries/{LIBRARY_ID_1}/", + exc=requests.exceptions.ConnectionError("connection refused"), + ) + with caplog.at_level(logging.WARNING, logger="datamasque.client.base"): + result = client.validate_discovery_config_library(config_library) + + assert result.is_valid is ValidationStatus.valid + assert any("Failed to clean up temporary validation library" in r.getMessage() for r in caplog.records) diff --git a/tests/test_discovery_configs.py b/tests/test_discovery_configs.py index caa4b31..367c3f2 100644 --- a/tests/test_discovery_configs.py +++ b/tests/test_discovery_configs.py @@ -1,13 +1,16 @@ """Tests for discovery-config support in the DataMasque client.""" +import logging from datetime import datetime from typing import Any import pytest +import requests import requests_mock +from pydantic import JsonValue, ValidationError from datamasque.client import DataMasqueClient -from datamasque.client.exceptions import DataMasqueApiError, DataMasqueException +from datamasque.client.exceptions import DataMasqueApiError, DataMasqueArgumentError, DataMasqueException from datamasque.client.models.discovery_config import ( DiscoveryConfig, DiscoveryConfigId, @@ -306,7 +309,17 @@ def test_update_discovery_config(client: DataMasqueClient, discovery_config: Dis def test_update_discovery_config_no_id_raises(client: DataMasqueClient, discovery_config: DiscoveryConfig) -> None: - with pytest.raises(ValueError, match="id is None"): + with pytest.raises(DataMasqueArgumentError, match="id is None"): + client.update_discovery_config(discovery_config) + + +def test_update_discovery_config_without_yaml_raises( + client: DataMasqueClient, discovery_config: DiscoveryConfig +) -> None: + discovery_config.id = DiscoveryConfigId(CONFIG_ID_1) + discovery_config.yaml = None + + with pytest.raises(DataMasqueArgumentError, match="without YAML content"): client.update_discovery_config(discovery_config) @@ -518,3 +531,193 @@ 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: JsonValue) -> dict[str, JsonValue]: + return { + "id": CONFIG_ID_1, + "name": "test_config", + "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_discovery_config_accepts_flat_details_list() -> None: + config = DiscoveryConfig.model_validate( + _build_config_response( + "invalid", + validation_error_details=[{"message": "Unknown mask 'foo'", "line_number": 3}], + ) + ) + + assert [detail.message for detail in config.validation_error_details] == ["Unknown mask 'foo'"] + + +def test_discovery_config_accepts_empty_errors_map() -> None: + config = DiscoveryConfig.model_validate(_build_config_response("valid", errors={})) + + assert config.validation_error_details == [] + + +def test_discovery_config_rejects_non_list_error_group() -> None: + with pytest.raises(ValidationError, match="config_yaml"): + DiscoveryConfig.model_validate(_build_config_response("invalid", errors={"config_yaml": "Unknown mask 'foo'"})) + + +def test_discovery_config_rejects_null_error_group() -> None: + with pytest.raises(ValidationError, match="config_yaml"): + DiscoveryConfig.model_validate(_build_config_response("invalid", errors={"config_yaml": None})) + + +def test_discovery_config_rejects_malformed_error_entry() -> None: + with pytest.raises(ValidationError): + DiscoveryConfig.model_validate( + _build_config_response("invalid", errors={"config_yaml": ["Unknown mask 'foo'"]}) + ) + + +def test_validate_discovery_config_round_trips_temp_copy( + 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) + delete = 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 discovery_config + assert result.is_valid is ValidationStatus.valid + assert result.validation_error is None + assert result.id is None + assert delete.called_once + + request_body = m.request_history[0].json() + assert request_body["name"].startswith("dm_python_validate_") + assert request_body["config_yaml"] == discovery_config.yaml + + +def test_validate_discovery_config_surfaces_structured_errors( + client: DataMasqueClient, discovery_config: DiscoveryConfig +) -> None: + invalid_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=invalid_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 [detail.line_number for detail in result.validation_error_details] == [3] + + +def test_validate_discovery_config_returns_outcome_when_cleanup_fails( + 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=500, json={}) + result = client.validate_discovery_config(discovery_config) + + assert result.is_valid is ValidationStatus.valid + + +def test_validate_discovery_config_without_yaml_raises( + client: DataMasqueClient, sample_config_list_response: dict[str, Any] +) -> None: + + with requests_mock.Mocker() as m: + m.get("http://test-server/api/discovery/configs/", json=sample_config_list_response, status_code=200) + config = client.list_discovery_configs()[0] + assert config.yaml is None + + with pytest.raises(DataMasqueArgumentError, match="without YAML content"): + client.validate_discovery_config(config) + + assert not any(request.method == "POST" for request in m.request_history) + + +def test_create_discovery_config_with_empty_yaml_raises(client: DataMasqueClient) -> None: + config = DiscoveryConfig(name="test_config", yaml="", config_type="database") + + with requests_mock.Mocker() as m: + with pytest.raises(DataMasqueArgumentError, match="without YAML content"): + client.create_discovery_config(config) + + assert not any(request.method == "POST" for request in m.request_history) + + +def test_validate_discovery_config_with_empty_yaml_raises(client: DataMasqueClient) -> None: + config = DiscoveryConfig(name="test_config", yaml="", config_type="database") + + with requests_mock.Mocker() as m: + with pytest.raises(DataMasqueArgumentError, match="without YAML content"): + client.validate_discovery_config(config) + + assert not any(request.method == "POST" for request in m.request_history) + + +def test_update_discovery_config_with_empty_yaml_raises(client: DataMasqueClient) -> None: + config = DiscoveryConfig(id=CONFIG_ID_1, name="test_config", yaml="", config_type="database") + + with requests_mock.Mocker() as m: + with pytest.raises(DataMasqueArgumentError, match="without YAML content"): + client.update_discovery_config(config) + + assert not any(request.method == "PUT" for request in m.request_history) + + +def test_validate_discovery_config_deletes_temp_config_when_response_is_rejected( + client: DataMasqueClient, discovery_config: DiscoveryConfig +) -> None: + with requests_mock.Mocker() as m: + m.post( + "http://test-server/api/discovery/configs/", + json=_build_config_response("not_a_real_status"), + status_code=201, + ) + delete = m.delete(f"http://test-server/api/discovery/configs/{CONFIG_ID_1}/", status_code=204) + + with pytest.raises(ValidationError): + client.validate_discovery_config(discovery_config) + + assert delete.called_once + + +def test_validate_discovery_config_returns_outcome_when_cleanup_cannot_reach_server( + client: DataMasqueClient, discovery_config: DiscoveryConfig, caplog: pytest.LogCaptureFixture +) -> 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}/", + exc=requests.exceptions.ConnectionError("connection refused"), + ) + with caplog.at_level(logging.WARNING, logger="datamasque.client.base"): + result = client.validate_discovery_config(discovery_config) + + assert result.is_valid is ValidationStatus.valid + assert any("Failed to clean up temporary validation config" in r.getMessage() for r in caplog.records) From 40292215b63c5caa7eec233f93b5e3882b9aedf0 Mon Sep 17 00:00:00 2001 From: Peter <101368063+ClassicMMT@users.noreply.github.com> Date: Fri, 31 Jul 2026 10:34:09 +1200 Subject: [PATCH 2/3] fix: address review - Move temp file logic to cli - Removed DataMasqueArgumentError --- HISTORY.rst | 7 +- datamasque/client/base.py | 7 - .../client/discovery_config_libraries.py | 48 +------ datamasque/client/discovery_configs.py | 45 +------ datamasque/client/exceptions.py | 10 -- tests/test_discovery_config_libraries.py | 122 +----------------- tests/test_discovery_configs.py | 119 +---------------- 7 files changed, 21 insertions(+), 337 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 349e5ef..08aaa5c 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -7,10 +7,9 @@ History * Added ``validation_error_details`` to ``DiscoveryConfig``. * Added ``usage_count`` to ``DiscoveryConfigLibrary``. -* Added ``validate_discovery_config`` and ``validate_discovery_config_library``. -* ``update_discovery_config``, ``validate_discovery_config``, and ``validate_discovery_config_library`` -* Now raise ``DataMasqueArgumentError`` when the passed entity has no ``yaml`` content, - instead of sending a request the server rejects. +* ``create_discovery_config``, ``update_discovery_config``, ``create_discovery_config_library``, and + ``update_discovery_config_library`` now raise ``ValueError`` when the passed entity has no ``yaml`` + content, instead of sending a request the server rejects. 1.2.1 (2026-07-30) ------------------ diff --git a/datamasque/client/base.py b/datamasque/client/base.py index 3b045bb..9aa3bea 100644 --- a/datamasque/client/base.py +++ b/datamasque/client/base.py @@ -17,7 +17,6 @@ from datamasque.client.exceptions import ( DataMasqueApiError, - DataMasqueException, DataMasqueNotReadyError, DataMasqueTransportError, ) @@ -310,12 +309,6 @@ def _delete_if_exists(self, path: str, *, params: Optional[dict] = None) -> None self._raise_for_status(response) - def _delete_best_effort(self, delete: Callable[[], None], description: str) -> None: - try: - delete() - except DataMasqueException as e: - logger.warning("Failed to clean up %s; remove it manually. Error: %s", description, e) - def _iter_paginated( self, path: str, diff --git a/datamasque/client/discovery_config_libraries.py b/datamasque/client/discovery_config_libraries.py index a355ffb..bd1e1fb 100644 --- a/datamasque/client/discovery_config_libraries.py +++ b/datamasque/client/discovery_config_libraries.py @@ -1,9 +1,8 @@ import logging -import uuid from typing import Optional from datamasque.client.base import BaseClient -from datamasque.client.exceptions import DataMasqueApiError, DataMasqueArgumentError +from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.discovery_config_library import DiscoveryConfigLibrary, DiscoveryConfigLibraryId logger = logging.getLogger(__name__) @@ -81,9 +80,7 @@ def create_discovery_config_library(self, library: DiscoveryConfigLibrary) -> Di """ if not library.yaml: - raise DataMasqueArgumentError( - "Cannot create a discovery config library without YAML content (yaml is empty)" - ) + raise ValueError("Cannot create a discovery config library without YAML content (yaml is empty)") data = library.model_dump(exclude_none=True, by_alias=True, mode="json") response = self.make_request("POST", "/api/discovery/config-libraries/", data=data) @@ -106,12 +103,10 @@ def update_discovery_config_library(self, library: DiscoveryConfigLibrary) -> Di """ if library.id is None: - raise DataMasqueArgumentError( - "Cannot update a discovery config library that has not been created yet (id is None)" - ) + raise ValueError("Cannot update a discovery config library that has not been created yet (id is None)") if not library.yaml: - raise DataMasqueArgumentError( + raise ValueError( "Cannot update a discovery config library without YAML content (yaml is empty or unset); " "list results omit YAML, so fetch the full library with `get_discovery_config_library` first" ) @@ -140,41 +135,6 @@ 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) -> DiscoveryConfigLibrary: - """Validates a discovery config library against the server without persisting it.""" - - if not library.yaml: - raise DataMasqueArgumentError( - "Cannot validate a discovery config library without YAML content (yaml is empty or unset); " - "list results omit YAML, so fetch the full library with `get_discovery_config_library` first" - ) - - temporary = DiscoveryConfigLibrary( - name=f"dm_python_validate_{uuid.uuid4().hex}", - namespace=library.namespace, - yaml=library.yaml, - ) - data = temporary.model_dump(exclude_none=True, by_alias=True, mode="json") - response = self.make_request("POST", "/api/discovery/config-libraries/", data=data) - - payload = response.json() - raw_id = payload.get("id") if isinstance(payload, dict) else None - created_id = DiscoveryConfigLibraryId(raw_id) if isinstance(raw_id, str) else None - - try: - created = DiscoveryConfigLibrary.model_validate(payload) - library.is_valid = created.is_valid - library.validation_error = created.validation_error - finally: - if created_id is not None: - self._delete_best_effort( - lambda: self.delete_discovery_config_library_by_id_if_exists(created_id), - f'temporary validation library "{temporary.name}"', - ) - - logger.debug('Validation of discovery config library "%s" complete', library.name) - 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 7226e30..d19f900 100644 --- a/datamasque/client/discovery_configs.py +++ b/datamasque/client/discovery_configs.py @@ -1,9 +1,8 @@ import logging -import uuid from typing import Iterator, Optional from datamasque.client.base import BaseClient -from datamasque.client.exceptions import DataMasqueApiError, DataMasqueArgumentError, DataMasqueException +from datamasque.client.exceptions import DataMasqueApiError, DataMasqueException from datamasque.client.models.discovery_config import DiscoveryConfig, DiscoveryConfigId, DiscoveryConfigType from datamasque.client.models.pagination import Page @@ -81,7 +80,7 @@ def create_discovery_config(self, config: DiscoveryConfig) -> DiscoveryConfig: """ if not config.yaml: - raise DataMasqueArgumentError("Cannot create a discovery config without YAML content (yaml is empty)") + raise ValueError("Cannot create a discovery config without YAML content (yaml is empty)") data = config.model_dump(exclude_none=True, by_alias=True, mode="json") response = self.make_request("POST", "/api/discovery/configs/", data=data) @@ -104,10 +103,10 @@ def update_discovery_config(self, config: DiscoveryConfig) -> DiscoveryConfig: """ if config.id is None: - raise DataMasqueArgumentError("Cannot update a discovery config that has not been created yet (id is None)") + raise ValueError("Cannot update a discovery config that has not been created yet (id is None)") if not config.yaml: - raise DataMasqueArgumentError( + raise ValueError( "Cannot update a discovery config without YAML content (yaml is empty or unset); " "list results omit YAML, so fetch the full config with `get_discovery_config` first" ) @@ -136,42 +135,6 @@ def create_or_update_discovery_config(self, config: DiscoveryConfig) -> Discover return self.create_discovery_config(config) - def validate_discovery_config(self, config: DiscoveryConfig) -> DiscoveryConfig: - """Validates a discovery config against the server.""" - - if not config.yaml: - raise DataMasqueArgumentError( - "Cannot validate a discovery config without YAML content (yaml is empty or unset); " - "list results omit YAML, so fetch the full config with `get_discovery_config` first" - ) - - temporary = DiscoveryConfig( - name=f"dm_python_validate_{uuid.uuid4().hex}", - yaml=config.yaml, - config_type=config.config_type, - ) - data = temporary.model_dump(exclude_none=True, by_alias=True, mode="json") - response = self.make_request("POST", "/api/discovery/configs/", data=data) - - payload = response.json() - raw_id = payload.get("id") if isinstance(payload, dict) else None - created_id = DiscoveryConfigId(raw_id) if isinstance(raw_id, str) else None - - try: - created = DiscoveryConfig.model_validate(payload) - config.is_valid = created.is_valid - config.validation_error = created.validation_error - config.validation_error_details = created.validation_error_details - finally: - if created_id is not None: - self._delete_best_effort( - lambda: self.delete_discovery_config_by_id_if_exists(created_id), - f'temporary validation config "{temporary.name}"', - ) - - logger.debug('Validation of discovery config "%s" complete', config.name) - 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/exceptions.py b/datamasque/client/exceptions.py index d24cb59..9943b8b 100644 --- a/datamasque/client/exceptions.py +++ b/datamasque/client/exceptions.py @@ -9,16 +9,6 @@ class DataMasqueUserError(DataMasqueException): """Raised when error occurs during user creation or configuration.""" -class DataMasqueArgumentError(DataMasqueException): - """ - Raised when a client method is given an object it cannot act on. - - Covers arguments the client rejects without contacting the server, such as - updating a record that has no `id` yet, or sending a config whose `yaml` is - empty. - """ - - class DataMasqueApiError(DataMasqueException): """ Raised when the DataMasque server responds to a request with a non-2xx status code. diff --git a/tests/test_discovery_config_libraries.py b/tests/test_discovery_config_libraries.py index f733cef..20a0d4d 100644 --- a/tests/test_discovery_config_libraries.py +++ b/tests/test_discovery_config_libraries.py @@ -1,20 +1,17 @@ """Tests for discovery config library support in the DataMasque client.""" -import logging from datetime import datetime -from typing import Any, Optional +from typing import Any import pytest -import requests import requests_mock -from pydantic import ValidationError from datamasque.client import ( DataMasqueClient, DiscoveryConfigLibrary, DiscoveryConfigLibraryId, ) -from datamasque.client.exceptions import DataMasqueApiError, DataMasqueArgumentError +from datamasque.client.exceptions import DataMasqueApiError from datamasque.client.models.status import ValidationStatus LIBRARY_ID_1 = "aaaaaaaa-1111-2222-3333-444444444444" @@ -349,7 +346,7 @@ def test_update_discovery_config_library(client: DataMasqueClient, config_librar def test_update_discovery_config_library_no_id_raises( client: DataMasqueClient, config_library: DiscoveryConfigLibrary ) -> None: - with pytest.raises(DataMasqueArgumentError, match="id is None"): + with pytest.raises(ValueError, match="id is None"): client.update_discovery_config_library(config_library) @@ -359,7 +356,7 @@ def test_update_discovery_config_library_without_yaml_raises( config_library.id = DiscoveryConfigLibraryId(LIBRARY_ID_1) config_library.yaml = None - with pytest.raises(DataMasqueArgumentError, match="without YAML content"): + with pytest.raises(ValueError, match="without YAML content"): client.update_discovery_config_library(config_library) @@ -505,120 +502,11 @@ def test_delete_discovery_config_library_by_name_not_found( assert m.request_history[0].method == "GET" -@pytest.mark.parametrize( - ("is_valid", "validation_error"), - [("valid", None), ("invalid", 'duplicate label "email"')], -) -def test_validate_discovery_config_library_round_trips_outcome( - client: DataMasqueClient, - config_library: DiscoveryConfigLibrary, - is_valid: str, - validation_error: Optional[str], -) -> None: - create_response = { - "id": LIBRARY_ID_1, - "name": "dm_python_validate_abc", - "namespace": "test_ns", - "is_valid": is_valid, - "validation_error": validation_error, - "usage_count": 0, - } - - with requests_mock.Mocker() as m: - m.post("http://test-server/api/discovery/config-libraries/", json=create_response, status_code=201) - delete = m.delete(f"http://test-server/api/discovery/config-libraries/{LIBRARY_ID_1}/", status_code=204) - result = client.validate_discovery_config_library(config_library) - - assert result is config_library - assert result.is_valid is ValidationStatus(is_valid) - assert result.validation_error == validation_error - assert result.name == "test_library" - assert result.namespace == "test_ns" - assert delete.called_once - - request_body = m.request_history[0].json() - assert request_body["name"].startswith("dm_python_validate_") - assert request_body["namespace"] == "test_ns" - assert "config_yaml" in request_body - - -def test_validate_discovery_config_library_without_yaml_raises( - client: DataMasqueClient, sample_library_list_response: list[dict[str, Any]] -) -> None: - - with requests_mock.Mocker() as m: - m.get("http://test-server/api/discovery/config-libraries/", json=sample_library_list_response, status_code=200) - library = client.list_discovery_config_libraries()[0] - assert library.yaml is None - - with pytest.raises(DataMasqueArgumentError, match="without YAML content"): - client.validate_discovery_config_library(library) - - assert not any(request.method == "POST" for request in m.request_history) - - def test_create_discovery_config_library_with_empty_yaml_raises(client: DataMasqueClient) -> None: library = DiscoveryConfigLibrary(name="test_library", yaml="") with requests_mock.Mocker() as m: - with pytest.raises(DataMasqueArgumentError, match="without YAML content"): + with pytest.raises(ValueError, match="without YAML content"): client.create_discovery_config_library(library) assert not any(request.method == "POST" for request in m.request_history) - - -def test_validate_discovery_config_library_with_empty_yaml_raises(client: DataMasqueClient) -> None: - library = DiscoveryConfigLibrary(name="test_library", yaml="") - - with requests_mock.Mocker() as m: - with pytest.raises(DataMasqueArgumentError, match="without YAML content"): - client.validate_discovery_config_library(library) - - assert not any(request.method == "POST" for request in m.request_history) - - -def test_validate_discovery_config_library_deletes_temp_library_when_response_is_rejected( - client: DataMasqueClient, config_library: DiscoveryConfigLibrary -) -> None: - rejected_response = { - "id": LIBRARY_ID_1, - "name": "dm_python_validate_abc", - "namespace": "", - "is_valid": "not_a_real_status", - "validation_error": None, - "usage_count": 0, - } - - with requests_mock.Mocker() as m: - m.post("http://test-server/api/discovery/config-libraries/", json=rejected_response, status_code=201) - delete = m.delete(f"http://test-server/api/discovery/config-libraries/{LIBRARY_ID_1}/", status_code=204) - - with pytest.raises(ValidationError): - client.validate_discovery_config_library(config_library) - - assert delete.called_once - - -def test_validate_discovery_config_library_returns_outcome_when_cleanup_cannot_reach_server( - client: DataMasqueClient, config_library: DiscoveryConfigLibrary, caplog: pytest.LogCaptureFixture -) -> None: - create_response = { - "id": LIBRARY_ID_1, - "name": "dm_python_validate_abc", - "namespace": "", - "is_valid": "valid", - "validation_error": None, - "usage_count": 0, - } - - with requests_mock.Mocker() as m: - m.post("http://test-server/api/discovery/config-libraries/", json=create_response, status_code=201) - m.delete( - f"http://test-server/api/discovery/config-libraries/{LIBRARY_ID_1}/", - exc=requests.exceptions.ConnectionError("connection refused"), - ) - with caplog.at_level(logging.WARNING, logger="datamasque.client.base"): - result = client.validate_discovery_config_library(config_library) - - assert result.is_valid is ValidationStatus.valid - assert any("Failed to clean up temporary validation library" in r.getMessage() for r in caplog.records) diff --git a/tests/test_discovery_configs.py b/tests/test_discovery_configs.py index 367c3f2..d34c47d 100644 --- a/tests/test_discovery_configs.py +++ b/tests/test_discovery_configs.py @@ -1,16 +1,14 @@ """Tests for discovery-config support in the DataMasque client.""" -import logging from datetime import datetime from typing import Any import pytest -import requests import requests_mock from pydantic import JsonValue, ValidationError from datamasque.client import DataMasqueClient -from datamasque.client.exceptions import DataMasqueApiError, DataMasqueArgumentError, DataMasqueException +from datamasque.client.exceptions import DataMasqueApiError, DataMasqueException from datamasque.client.models.discovery_config import ( DiscoveryConfig, DiscoveryConfigId, @@ -309,7 +307,7 @@ def test_update_discovery_config(client: DataMasqueClient, discovery_config: Dis def test_update_discovery_config_no_id_raises(client: DataMasqueClient, discovery_config: DiscoveryConfig) -> None: - with pytest.raises(DataMasqueArgumentError, match="id is None"): + with pytest.raises(ValueError, match="id is None"): client.update_discovery_config(discovery_config) @@ -319,7 +317,7 @@ def test_update_discovery_config_without_yaml_raises( discovery_config.id = DiscoveryConfigId(CONFIG_ID_1) discovery_config.yaml = None - with pytest.raises(DataMasqueArgumentError, match="without YAML content"): + with pytest.raises(ValueError, match="without YAML content"): client.update_discovery_config(discovery_config) @@ -596,128 +594,21 @@ def test_discovery_config_rejects_malformed_error_entry() -> None: ) -def test_validate_discovery_config_round_trips_temp_copy( - 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) - delete = 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 discovery_config - assert result.is_valid is ValidationStatus.valid - assert result.validation_error is None - assert result.id is None - assert delete.called_once - - request_body = m.request_history[0].json() - assert request_body["name"].startswith("dm_python_validate_") - assert request_body["config_yaml"] == discovery_config.yaml - - -def test_validate_discovery_config_surfaces_structured_errors( - client: DataMasqueClient, discovery_config: DiscoveryConfig -) -> None: - invalid_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=invalid_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 [detail.line_number for detail in result.validation_error_details] == [3] - - -def test_validate_discovery_config_returns_outcome_when_cleanup_fails( - 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=500, json={}) - result = client.validate_discovery_config(discovery_config) - - assert result.is_valid is ValidationStatus.valid - - -def test_validate_discovery_config_without_yaml_raises( - client: DataMasqueClient, sample_config_list_response: dict[str, Any] -) -> None: - - with requests_mock.Mocker() as m: - m.get("http://test-server/api/discovery/configs/", json=sample_config_list_response, status_code=200) - config = client.list_discovery_configs()[0] - assert config.yaml is None - - with pytest.raises(DataMasqueArgumentError, match="without YAML content"): - client.validate_discovery_config(config) - - assert not any(request.method == "POST" for request in m.request_history) - - def test_create_discovery_config_with_empty_yaml_raises(client: DataMasqueClient) -> None: config = DiscoveryConfig(name="test_config", yaml="", config_type="database") with requests_mock.Mocker() as m: - with pytest.raises(DataMasqueArgumentError, match="without YAML content"): + with pytest.raises(ValueError, match="without YAML content"): client.create_discovery_config(config) assert not any(request.method == "POST" for request in m.request_history) -def test_validate_discovery_config_with_empty_yaml_raises(client: DataMasqueClient) -> None: - config = DiscoveryConfig(name="test_config", yaml="", config_type="database") - - with requests_mock.Mocker() as m: - with pytest.raises(DataMasqueArgumentError, match="without YAML content"): - client.validate_discovery_config(config) - - assert not any(request.method == "POST" for request in m.request_history) - - def test_update_discovery_config_with_empty_yaml_raises(client: DataMasqueClient) -> None: config = DiscoveryConfig(id=CONFIG_ID_1, name="test_config", yaml="", config_type="database") with requests_mock.Mocker() as m: - with pytest.raises(DataMasqueArgumentError, match="without YAML content"): + with pytest.raises(ValueError, match="without YAML content"): client.update_discovery_config(config) assert not any(request.method == "PUT" for request in m.request_history) - - -def test_validate_discovery_config_deletes_temp_config_when_response_is_rejected( - client: DataMasqueClient, discovery_config: DiscoveryConfig -) -> None: - with requests_mock.Mocker() as m: - m.post( - "http://test-server/api/discovery/configs/", - json=_build_config_response("not_a_real_status"), - status_code=201, - ) - delete = m.delete(f"http://test-server/api/discovery/configs/{CONFIG_ID_1}/", status_code=204) - - with pytest.raises(ValidationError): - client.validate_discovery_config(discovery_config) - - assert delete.called_once - - -def test_validate_discovery_config_returns_outcome_when_cleanup_cannot_reach_server( - client: DataMasqueClient, discovery_config: DiscoveryConfig, caplog: pytest.LogCaptureFixture -) -> 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}/", - exc=requests.exceptions.ConnectionError("connection refused"), - ) - with caplog.at_level(logging.WARNING, logger="datamasque.client.base"): - result = client.validate_discovery_config(discovery_config) - - assert result.is_valid is ValidationStatus.valid - assert any("Failed to clean up temporary validation config" in r.getMessage() for r in caplog.records) From 60ea203d1887c0b9c18b6fe61dee45607c4792d1 Mon Sep 17 00:00:00 2001 From: Peter <101368063+ClassicMMT@users.noreply.github.com> Date: Fri, 31 Jul 2026 12:49:40 +1200 Subject: [PATCH 3/3] chore: bump version to 1.2.2 --- HISTORY.rst | 4 +++- pyproject.toml | 2 +- uv.lock | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 08aaa5c..453853e 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -2,7 +2,7 @@ History ======= -1.2.2 (unreleased) +1.2.2 (2026-07-31) ------------------ * Added ``validation_error_details`` to ``DiscoveryConfig``. @@ -11,6 +11,8 @@ History ``update_discovery_config_library`` now raise ``ValueError`` when the passed entity has no ``yaml`` content, instead of sending a request the server rejects. +Requires server version 3.26.14 + 1.2.1 (2026-07-30) ------------------ diff --git a/pyproject.toml b/pyproject.toml index 43aaab7..2945092 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "datamasque-python" -version = "1.2.1" +version = "1.2.2" description = "Official Python client for the DataMasque data-masking API." authors = [ { name = "DataMasque Ltd" }, diff --git a/uv.lock b/uv.lock index 0f6f752..1a32e7c 100644 --- a/uv.lock +++ b/uv.lock @@ -419,7 +419,7 @@ toml = [ [[package]] name = "datamasque-python" -version = "1.2.1" +version = "1.2.2" source = { editable = "." } dependencies = [ { name = "pydantic" },