Skip to content

Commit 67138ec

Browse files
Feature/pydantic serialization (#1302)
## Changes Follow-up to #1259, completing the Pydantic migration for the declarative checks path. Also addresses the review comment on #1259 about covering Pydantic `ValidationError` behaviour with tests: DQX callers never see raw `pydantic_core.ValidationError` — every entry point raises DQX error types with the pre-migration message format. **Validation & deserialization through Pydantic:** - New `CheckSpec` / `CheckBlock` Pydantic schemas in `checks_validator.py` own the structural validation of check metadata dicts (required `check` key, criticality enum, `for_each_column` non-empty list, argument dict shape). Signature-based argument validation (function exists, required params, types) is unchanged and runs after. - Pydantic errors are translated back into the pre-existing human-readable messages (`_translate_pydantic_error`), so users and existing tests see identical error strings. Unmatched errors name the offending field path (e.g. `user_metadata -> confidence`) instead of a generic message. Translation matches on stable Pydantic error *types*, not version-dependent message text. - `ChecksDeserializer.deserialize` reads typed attributes off the validated `CheckSpec` instead of raw-dict `.get()` calls, so schema, defaults, and validation live in one place. Note: it re-parses via `model_validate` after `validate_checks` — structural parsing is cheap, and having the validator return parsed specs would be a larger API change. - New `project_to_check_schema` strips storage-only columns (`run_config_name`, `created_at`, `rule_fingerprint`, `rule_set_fingerprint`) when loading checks from Lakebase, so stored checks round-trip cleanly into `apply_checks_by_metadata` and match the Delta path shape. The allow-list is derived from `CheckSpec.model_fields`. **`extra` policy on the migrated models (deliberate split):** - `extra="forbid"` on `DQRule`/`DQForEachColRule`/`BaseChecksStorageConfig`: a misspelled kwarg raises `InvalidCheckError`/`InvalidConfigError` at construction. This *restores released behaviour* — v0.15.0 (last release) predates the Pydantic migration and its dataclasses raised `TypeError` on unknown kwargs; the lenient `ignore` default was never shipped. - `extra="ignore"` on `CheckSpec`/`CheckBlock`: check *metadata* legitimately carries extra keys (storage columns, user files), and rejecting them would break the load -> apply round-trip. **Cleanup:** - Removed dead `checks_formats.py` (+ its tests): `FILE_SERIALIZERS`/`FILE_DESERIALIZERS` had no consumers anywhere in `src/` (already unused on `main`); `SerializerFactory` is the single source of truth for file formats. ### Linked issues Resolves #1285 ### Tests - [x] manually tested - [ ] added unit tests - [ ] added integration tests - [ ] added end-to-end tests - [ ] added performance tests New `tests/unit/test_pydantic_serialization.py` covers: - **Fingerprint byte-parity corpus**: SHA-256 values captured *before* the migration, asserted byte-identical after — stored fingerprints in existing tables stay valid. - Every translated Pydantic error path (missing `check`, bad criticality value *and* type, non-list/empty `for_each_column`, wrong-typed fields reported with their location). - YAML/JSON round-trip incl. the `__decimal__` wire format; deserializer dispatch to `DQRowRule`/`DQDatasetRule`/`for_each_column`; storage-column tolerance + projection. - Regression tests for `extra="forbid"` (unknown kwargs rejected, wrapped in DQX errors). --------- Co-authored-by: Marcin Wojtyczka <marcin.wojtyczka@databricks.com>
1 parent d2b8e33 commit 67138ec

13 files changed

Lines changed: 1415 additions & 304 deletions

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ src/databricks/labs/dqx/
6464
├── manager.py # DQRuleManager — build/manage rule collections
6565
├── config.py # WorkspaceConfig, RunConfig, AnomalyParams, LLMModelConfig, ExtraParams
6666
├── checks_storage.py # WorkspaceFileChecksStorageHandler, VolumeFileChecksStorageHandler
67-
├── checks_serializer.py / checks_resolver.py / checks_validator.py / checks_formats.py
67+
├── checks_serializer.py / checks_resolver.py / checks_validator.py
6868
├── config_serializer.py # ConfigSerializer — use instead of dataclasses.asdict()
6969
├── cli.py # Databricks Labs CLI commands (@dqx.command)
7070
├── errors.py # For example: MissingParameterError, InvalidParameterError, UnsafeSqlQueryError — use instead of built-in exceptions

src/databricks/labs/dqx/checks_formats.py

Lines changed: 0 additions & 18 deletions
This file was deleted.

src/databricks/labs/dqx/checks_serializer.py

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import yaml
99

1010
from databricks.labs.dqx.checks_resolver import resolve_check_function
11-
from databricks.labs.dqx.checks_validator import ChecksValidator
11+
from databricks.labs.dqx.checks_validator import ChecksValidator, CheckSpec
1212
from databricks.labs.dqx.rule import (
1313
DQRule,
1414
DQRowRule,
@@ -22,6 +22,31 @@
2222
logger = logging.getLogger(__name__)
2323

2424

25+
def project_to_check_schema(check: dict) -> dict:
26+
"""Return a copy of *check* containing only the logical check-metadata keys.
27+
28+
Storage backends persist columns alongside the check that are not part of the check metadata
29+
accepted by *apply_checks_by_metadata* (e.g. *run_config_name*, *created_at*, *rule_fingerprint*,
30+
*rule_set_fingerprint*). Loading a check therefore yields those extra keys. This helper drops
31+
them so a loaded check round-trips cleanly.
32+
33+
Both load paths funnel their assembled check dict through this single projection so they agree
34+
on the loaded shape: the Lakebase path (*LakebaseChecksStorageHandler._load_checks_from_lakebase*)
35+
and the Delta path (*DataFrameConverter.from_dataframe*). The allowed keys are derived from
36+
*CheckSpec.model_fields*, so a new logical field added to the schema is retained by both paths
37+
automatically — provided the backing store also carries it (adding a *CheckSpec* field still
38+
requires a matching Delta table column and Lakebase column for the value to survive a round-trip).
39+
40+
Args:
41+
check: A check dict that may carry storage-only keys.
42+
43+
Returns:
44+
A new dict with only the keys defined by *CheckSpec*.
45+
"""
46+
allowed_keys = set(CheckSpec.model_fields)
47+
return {key: value for key, value in check.items() if key in allowed_keys}
48+
49+
2550
class ChecksNormalizer:
2651
"""
2752
Handles normalization and denormalization of check dictionaries.
@@ -226,7 +251,7 @@ def __init__(self, custom_checks: dict[str, Callable] | None = None):
226251

227252
def deserialize(self, checks: list[dict]) -> list[DQRule]:
228253
"""
229-
Converts a list of quality checks defined as Python dictionaries to a list of `DQRule` objects.
254+
Converts a list of quality checks defined as Python dictionaries to a list of *DQRule* objects.
230255
231256
Args:
232257
checks: list of dictionaries describing checks. Each check is a dictionary
@@ -245,29 +270,30 @@ def deserialize(self, checks: list[dict]) -> list[DQRule]:
245270
Raises:
246271
InvalidCheckError: If any dictionary is invalid or unsupported.
247272
"""
248-
status = ChecksValidator.validate_checks(checks, self.custom_checks)
273+
status, specs = ChecksValidator.validate_and_parse(checks, self.custom_checks)
249274
if status.has_errors:
250275
raise InvalidCheckError(str(status))
251276

252277
dq_rule_checks: list[DQRule] = []
253-
for check_def in checks:
254-
logger.debug(f"Processing check definition: {check_def}")
278+
for spec in specs:
279+
# No errors means every check parsed, so specs holds a CheckSpec for each entry.
280+
assert spec is not None # validated above; narrows the type for the reads below
281+
logger.debug(f"Processing check definition: {spec}")
255282

256-
check = check_def.get("check", {})
257-
name = check_def.get("name") or ""
258-
func_name = check.get("function")
283+
func_name = spec.check.function
259284
func = resolve_check_function(func_name, self.custom_checks, fail_on_missing=True)
260285
assert func # should already be validated
261286

262-
func_args = check.get("arguments", {})
263-
for_each_column = check.get("for_each_column")
287+
func_args: dict[str, Any] = spec.check.arguments
288+
for_each_column = spec.check.for_each_column
264289
column = func_args.get("column") # should be defined for single-column checks only
265290
columns = func_args.get("columns") # should be defined for multi-column checks only
266291
assert not (column and columns) # should already be validated
267-
criticality = check_def.get("criticality", "error")
268-
filter_str = check_def.get("filter")
269-
user_metadata = check_def.get("user_metadata")
270-
message_expr = check_def.get("message_expr")
292+
name = spec.name or ""
293+
criticality = spec.criticality
294+
filter_str = spec.filter
295+
user_metadata = spec.user_metadata
296+
message_expr = spec.message_expr
271297

272298
# Exclude `column` and `columns` from check_func_kwargs
273299
# as these are always included in the check function call

src/databricks/labs/dqx/checks_storage.py

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
SerializerFactory,
6161
ChecksNormalizer,
6262
deserialize_checks,
63+
project_to_check_schema,
6364
)
6465
from databricks.labs.dqx.checks_validator import ChecksValidator
6566
from databricks.labs.dqx.rule_fingerprint import compute_rule_set_fingerprint_by_metadata
@@ -178,8 +179,10 @@ def from_dataframe(
178179
check_dict["filter"] = row.filter
179180
if row.user_metadata is not None:
180181
check_dict["user_metadata"] = row.user_metadata
181-
# Denormalize special markers back to objects
182-
checks.append(ChecksNormalizer.denormalize_value(check_dict))
182+
# Route through the same projection as the Lakebase load path so both loaders agree on
183+
# the loaded shape (the allow-list is derived from CheckSpec.model_fields). Then
184+
# denormalize special markers back to objects.
185+
checks.append(ChecksNormalizer.denormalize_value(project_to_check_schema(check_dict)))
183186
return checks
184187

185188
@staticmethod
@@ -535,7 +538,7 @@ def get_engine(self, config: LakebaseChecksStorageConfig) -> Engine:
535538
engine = create_engine(
536539
engine_url,
537540
pool_recycle=45 * 60, # recycle connections every 45 minutes
538-
connect_args={'sslmode': 'require'},
541+
connect_args={"sslmode": "require"},
539542
pool_size=4,
540543
)
541544
event.listen(engine, "do_connect", self._prepare_before_connect(config))
@@ -779,7 +782,7 @@ def _load_checks_from_lakebase(self, config: LakebaseChecksStorageConfig, engine
779782
f"for run_config_name='{config.run_config_name}'. "
780783
f"Make sure the profiler has run successfully and saved checks to this location."
781784
)
782-
checks_dict = [dict(check) for check in checks]
785+
checks_dict = [project_to_check_schema(dict(check)) for check in checks]
783786

784787
# Denormalize special markers back to objects
785788
return ChecksNormalizer.denormalize(checks_dict)
@@ -802,7 +805,7 @@ def _rule_set_columns_exists(engine: Engine, schema: str, table: str) -> bool:
802805
if not inspector.has_table(table, schema=schema):
803806
return False
804807
cols = inspector.get_columns(table, schema=schema)
805-
existing_column_names = {col['name'] for col in cols}
808+
existing_column_names = {col["name"] for col in cols}
806809
return all(x in existing_column_names for x in _VERSIONING_COLUMNS)
807810

808811
def _check_for_undefined_table_error(self, e: ProgrammingError, config: LakebaseChecksStorageConfig) -> NoReturn:
@@ -819,8 +822,8 @@ def _check_for_undefined_table_error(self, e: ProgrammingError, config: Lakebase
819822
NotFound: If the table does not exist in the Lakebase instance (pgcode 42P01).
820823
ProgrammingError: Re-raises the original error if it's not an undefined table error.
821824
"""
822-
pgcode = getattr(getattr(e, 'orig', None), 'pgcode', None)
823-
postgres_undefined_table_error = '42P01'
825+
pgcode = getattr(getattr(e, "orig", None), "pgcode", None)
826+
postgres_undefined_table_error = "42P01"
824827
if pgcode == postgres_undefined_table_error:
825828
raise NotFound(f"Table '{config.location}' does not exist in the Lakebase instance") from e
826829
raise e

0 commit comments

Comments
 (0)