From f641858384e656c4fdce71697320457fa556d9d5 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 5 Aug 2026 21:13:55 +0200 Subject: [PATCH] Name the check fields qualityId and failedSamples The test-results model spells every other field in camelCase (runId, dataContractId, timestampStart), so the two check fields that carried the Python attribute name now do too. The old names stay available: they are accepted as input (keyword arguments and test results serialized by an older version), still read and write on the Check model with a DeprecationWarning, and are still written next to the new ones, so a consumer of /api/test-results keeps working. An unset field is not serialized under either name. --- CHANGELOG.md | 1 + .../datacontract/check_azure_blob_file.py | 2 +- .../engines/ibis/ibis_check_execute.py | 4 +- datacontract/model/run.py | 56 ++++++++++++++-- datacontract/output/junit_test_results.py | 4 +- docs/docs/release-notes.md | 1 + tests/test_run_check_deprecated_fields.py | 65 +++++++++++++++++++ tests/test_test_azure_blob_file.py | 2 +- tests/test_test_failed_samples.py | 16 ++--- tests/test_test_quality_id_and_tag_filter.py | 6 +- 10 files changed, 135 insertions(+), 22 deletions(-) create mode 100644 tests/test_run_check_deprecated_fields.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 78a0c6c38..68260e7cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Changed +- Test results name the check fields `qualityId` and `failedSamples` instead of `quality_id` and `failed_samples`; the old names are deprecated, but still accepted as input and still written next to the new ones - `datacontract import unity` no longer writes the `databricksType` custom property, which duplicated `physicalType` - `datacontract export sql --server databricks` keeps the declared length of `varchar(n)` and `char(n)` instead of exporting `STRING` diff --git a/datacontract/engines/datacontract/check_azure_blob_file.py b/datacontract/engines/datacontract/check_azure_blob_file.py index 4f0cd5c19..1ac13a07b 100644 --- a/datacontract/engines/datacontract/check_azure_blob_file.py +++ b/datacontract/engines/datacontract/check_azure_blob_file.py @@ -629,7 +629,7 @@ def _append_check( name=name, model=model, field=field, - quality_id=quality_id, + qualityId=quality_id, tags=tags, engine="datacontract", language="python", diff --git a/datacontract/engines/ibis/ibis_check_execute.py b/datacontract/engines/ibis/ibis_check_execute.py index e6e11f617..bab066d4e 100644 --- a/datacontract/engines/ibis/ibis_check_execute.py +++ b/datacontract/engines/ibis/ibis_check_execute.py @@ -66,7 +66,7 @@ def build_check_stubs(specs: List[CheckSpec]) -> List[Check]: name=spec.name, model=spec.model, field=spec.field, - quality_id=spec.quality_id, + qualityId=spec.quality_id, tags=spec.tags, engine="ibis", implementation=_describe(spec), @@ -365,7 +365,7 @@ def _collect_failed_samples(run, t, columns, schema, model, specs, data_contract logger.debug("Could not collect failed samples for '%s': %s", spec.key, e) continue if samples: - check.failed_samples = samples + check.failedSamples = samples def _sample_field_meta(data_contract, server, model): diff --git a/datacontract/model/run.py b/datacontract/model/run.py index 6ae3be442..fade25ea8 100644 --- a/datacontract/model/run.py +++ b/datacontract/model/run.py @@ -1,11 +1,12 @@ import logging +import warnings from datetime import datetime, timezone from enum import Enum from importlib import metadata from typing import List from uuid import UUID, uuid4 -from pydantic import BaseModel +from pydantic import AliasChoices, BaseModel, Field, SerializerFunctionWrapHandler, model_serializer def _cli_version() -> str: @@ -15,6 +16,32 @@ def _cli_version() -> str: return "unknown" +# Check fields renamed to camelCase, mapped to the snake_case name they had +# before. Both spellings are accepted as input, readable on the model, and +# written to the test results. +_DEPRECATED_CHECK_ALIASES = {"qualityId": "quality_id", "failedSamples": "failed_samples"} + + +def _deprecated_alias(old_name: str, new_name: str) -> property: + """Read/write access to a renamed check field under its former snake_case name. + + The check fields were renamed to camelCase to match the rest of the + test-results model (``runId``, ``dataContractId``, ...). Code written against + the old names keeps working and warns. + """ + message = f"Check.{old_name} is deprecated, use Check.{new_name} instead." + + def getter(self): + warnings.warn(message, DeprecationWarning, stacklevel=2) + return getattr(self, new_name) + + def setter(self, value): + warnings.warn(message, DeprecationWarning, stacklevel=2) + setattr(self, new_name, value) + + return property(getter, setter, doc=message) + + class ResultEnum(str, Enum): passed = "passed" warning = "warning" @@ -30,12 +57,12 @@ class Check(BaseModel): category: str | None = None type: str name: str | None = None - model: str | None = None - field: str | None = None + model: str | None = None # naming for historic reasons. Should rather be named schema + field: str | None = None # naming for historic reasons. Should rather be named property # The ODCS `quality.id` / `quality.tags` of the rule this check comes from, # so a check can be traced back to (and re-run through `test --quality-id` # / `test --tag`) the rule that declared it. Empty for built-in checks. - quality_id: str | None = None + qualityId: str | None = Field(default=None, validation_alias=AliasChoices("qualityId", "quality_id")) tags: list[str] | None = None engine: str | None = None @@ -49,7 +76,26 @@ class Check(BaseModel): # `datacontract test --include-failed-samples` is set). Each entry is a row # restricted to identifier + offending columns, with sensitive columns # omitted. The full failed count lives in `diagnostics`, not here. - failed_samples: list | None = None + failedSamples: list | None = Field(default=None, validation_alias=AliasChoices("failedSamples", "failed_samples")) + + # Deprecated former names of the two fields above. They still read, write and + # validate, so `check.failed_samples` and older test-results JSON keep working. + quality_id = _deprecated_alias("quality_id", "qualityId") + failed_samples = _deprecated_alias("failed_samples", "failedSamples") + + @model_serializer(mode="wrap") + def _serialize_with_deprecated_aliases(self, handler: SerializerFunctionWrapHandler) -> dict: + """Write a set field under its deprecated name too. + + The test results are published to the `/api/test-results` API, so a + consumer still reading `quality_id` / `failed_samples` keeps working. A + field that is not set stays absent under both names. + """ + data = handler(self) + for new_name, old_name in _DEPRECATED_CHECK_ALIASES.items(): + if new_name in data and getattr(self, new_name) is not None: + data[old_name] = data[new_name] + return data class Log(BaseModel): diff --git a/datacontract/output/junit_test_results.py b/datacontract/output/junit_test_results.py index e40448698..dad01c1c5 100644 --- a/datacontract/output/junit_test_results.py +++ b/datacontract/output/junit_test_results.py @@ -123,8 +123,8 @@ def to_failure_text(check): f"Reason: {check.reason}\n" f"Diagnostics:\n{yaml.dump(check.diagnostics, default_flow_style=False)}" ) - if check.failed_samples: - text += f"Failed samples:\n{yaml.dump(check.failed_samples, default_flow_style=False)}" + if check.failedSamples: + text += f"Failed samples:\n{yaml.dump(check.failedSamples, default_flow_style=False)}" return text diff --git a/docs/docs/release-notes.md b/docs/docs/release-notes.md index 4f4646002..31716a04d 100644 --- a/docs/docs/release-notes.md +++ b/docs/docs/release-notes.md @@ -27,6 +27,7 @@ marked as such in the entry. ## Unreleased {#unreleased} ### Changed +- Test results name the check fields `qualityId` and `failedSamples` instead of `quality_id` and `failed_samples`; the old names are deprecated, but still accepted as input and still written next to the new ones - `datacontract import unity` no longer writes the `databricksType` custom property, which duplicated `physicalType` - `datacontract export sql --server databricks` keeps the declared length of `varchar(n)` and `char(n)` instead of exporting `STRING` diff --git a/tests/test_run_check_deprecated_fields.py b/tests/test_run_check_deprecated_fields.py new file mode 100644 index 000000000..fb092cd45 --- /dev/null +++ b/tests/test_run_check_deprecated_fields.py @@ -0,0 +1,65 @@ +"""The check fields renamed to camelCase keep working under their old names. + +`quality_id` / `failed_samples` were renamed to `qualityId` / `failedSamples` to +match the rest of the test-results model (`runId`, `dataContractId`, ...). Code +and stored test results written against the old names must keep working. +""" + +import pytest + +from datacontract.model.run import Check + + +def test_reading_the_old_name_returns_the_new_value(): + check = Check(type="field_type", qualityId="orders_not_empty", failedSamples=[{"id": 1}]) + + with pytest.warns(DeprecationWarning, match="Check.quality_id is deprecated"): + assert check.quality_id == "orders_not_empty" + with pytest.warns(DeprecationWarning, match="Check.failed_samples is deprecated"): + assert check.failed_samples == [{"id": 1}] + + +def test_writing_the_old_name_sets_the_new_field(): + check = Check(type="field_type") + + with pytest.warns(DeprecationWarning, match="Check.quality_id is deprecated"): + check.quality_id = "orders_not_empty" + with pytest.warns(DeprecationWarning, match="Check.failed_samples is deprecated"): + check.failed_samples = [{"id": 1}] + + assert check.qualityId == "orders_not_empty" + assert check.failedSamples == [{"id": 1}] + + +def test_the_old_names_are_still_accepted_as_input(): + # keyword construction, and test results serialized by an older version + assert Check(type="field_type", quality_id="r", failed_samples=[1]).qualityId == "r" + restored = Check.model_validate({"type": "field_type", "quality_id": "r", "failed_samples": [1]}) + assert (restored.qualityId, restored.failedSamples) == ("r", [1]) + + +def test_a_set_field_is_serialized_under_both_names(): + # the test results are published to /api/test-results, so a consumer still + # reading the old names keeps working + check = Check(type="field_type", quality_id="r", failed_samples=[1]) + + serialized = check.model_dump(exclude_none=True) + + assert serialized == { + "type": "field_type", + "qualityId": "r", + "failedSamples": [1], + "quality_id": "r", + "failed_samples": [1], + } + + +def test_an_unset_field_is_not_serialized_under_the_old_name(): + check = Check(type="field_type") + + serialized = check.model_dump() + + assert serialized["qualityId"] is None + assert serialized["failedSamples"] is None + assert "quality_id" not in serialized + assert "failed_samples" not in serialized diff --git a/tests/test_test_azure_blob_file.py b/tests/test_test_azure_blob_file.py index 55028759a..479e36d0a 100644 --- a/tests/test_test_azure_blob_file.py +++ b/tests/test_test_azure_blob_file.py @@ -418,7 +418,7 @@ def _run(self, **kwargs) -> Run: def test_quality_id_selects_a_single_rule(self): run = self._run(quality_ids={"enough_files"}) assert [c.type for c in run.checks] == ["azure_file_count_quality"] - assert run.checks[0].quality_id == "enough_files" + assert run.checks[0].qualityId == "enough_files" def test_tag_selects_every_rule_carrying_it(self): run = self._run(tags={"critical"}) diff --git a/tests/test_test_failed_samples.py b/tests/test_test_failed_samples.py index 258f3e727..cb719cab7 100644 --- a/tests/test_test_failed_samples.py +++ b/tests/test_test_failed_samples.py @@ -58,17 +58,17 @@ def _check(run, type_, field=None): def test_no_samples_collected_without_flag(): run = _run(include_failed_samples=False) assert run.result == ResultEnum.failed # there are real violations - assert all(c.failed_samples is None for c in run.checks) + assert all(c.failedSamples is None for c in run.checks) def test_missing_samples_have_identifier_and_offending_column(): run = _run(include_failed_samples=True) check = _check(run, "field_required", field="region") assert check.result == ResultEnum.failed - assert check.failed_samples is not None + assert check.failedSamples is not None # region is empty for ids 3 and 5. - assert {s["id"] for s in check.failed_samples} == {3, 5} - for s in check.failed_samples: + assert {s["id"] for s in check.failedSamples} == {3, 5} + for s in check.failedSamples: assert set(s.keys()) == {"id", "region"} assert s["region"] is None @@ -77,7 +77,7 @@ def test_invalid_range_sample_includes_offending_value(): run = _run(include_failed_samples=True) check = _check(run, "field_maximum", field="amount") assert check.result == ResultEnum.failed - assert check.failed_samples == [{"id": 3, "amount": 200}] + assert check.failedSamples == [{"id": 3, "amount": 200}] def test_samples_respect_the_limit(): @@ -85,14 +85,14 @@ def test_samples_respect_the_limit(): check = _check(run, "field_regex", field="email") assert check.result == ResultEnum.failed # 6 rows fail the email pattern, but samples are capped at 5. - assert len(check.failed_samples) == 5 + assert len(check.failedSamples) == 5 def test_sensitive_column_is_omitted_from_samples(): run = _run(include_failed_samples=True) check = _check(run, "field_regex", field="email") # email is classified PII, so its value must not appear; only the identifier. - for s in check.failed_samples: + for s in check.failedSamples: assert "email" not in s assert set(s.keys()) == {"id"} @@ -101,4 +101,4 @@ def test_duplicate_samples_report_key_and_count(): run = _run(include_failed_samples=True) check = _check(run, "field_unique", field="id") assert check.result == ResultEnum.failed - assert check.failed_samples == [{"id": 2, "duplicate_count": 2}] + assert check.failedSamples == [{"id": 2, "duplicate_count": 2}] diff --git a/tests/test_test_quality_id_and_tag_filter.py b/tests/test_test_quality_id_and_tag_filter.py index 2a9f2fc1b..82ec440d4 100644 --- a/tests/test_test_quality_id_and_tag_filter.py +++ b/tests/test_test_quality_id_and_tag_filter.py @@ -123,11 +123,11 @@ def test_checks_report_the_id_and_tags_of_their_rule(): run = DataContract(data_contract_str=CONTRACT).test() print(run.pretty()) by_type = {check.type: check for check in run.checks} - assert by_type["row_count"].quality_id == "orders_not_empty" + assert by_type["row_count"].qualityId == "orders_not_empty" assert by_type["row_count"].tags == ["critical", "cheap"] - assert by_type["field_null_values"].quality_id is None + assert by_type["field_null_values"].qualityId is None assert by_type["field_null_values"].tags is None - assert by_type["field_is_present"].quality_id is None + assert by_type["field_is_present"].qualityId is None def test_quality_id_cli_option():