Skip to content

Improve PydanticValidationError to take a pydantic.ValidationError instead of a raw error-dict list #430

Description

@candleindark

Summary

PydanticValidationError.__init__ currently accepts errors: List[Dict[str, Any]], but the actual contract is narrower and undocumented: the list is assumed to be the pydantic_core.ErrorDetails list returned by pydantic.ValidationError.errors() (https://docs.pydantic.dev/latest/errors/errors/). Nothing in the type signature enforces this, so the constructor's real precondition is invisible to callers and to mypy.

Proposed change

class PydanticValidationError(ValidationError):
    """Validation errors were detected by pydantic"""

    def __init__(self, ve: pydantic.ValidationError) -> None:
        self.errors: list[pydantic_core.ErrorDetails] = ve.errors()

Taking the raw pydantic.ValidationError makes the input contract explicit while leaving the constructed object unchanged: self.errors is still the same list[ErrorDetails] that consumers already read.

The same PR should also tighten the annotation on the receiving side in dandischema/tests/test_metadata.py (e.g. the comprehension over exc.value.errors around line 319) to pydantic_core.ErrorDetails, rather than leaving it as an untyped/Any value.

This should land after #422 merges, since that PR removes the missing_ok filtering in dandischema/metadata.py that currently sits between catching the exception and constructing PydanticValidationError; a constructor that recomputes ve.errors() internally can't reproduce that filtering.

Investigation details: sequencing rationale and cross-repo impact check

Sequencing: today, validate() catches pydantic.ValidationError, filters out "missing"-type entries when missing_ok=True, and only then builds PydanticValidationError from the filtered list. Once #422 removes missing_ok and that filtering branch, the call site becomes a straight pass-through of the caught exception, which is what this change assumes.

Cross-repo impact check (against local clones of dandi-cli and dandi-archive):

  • dandi-cli never imports or touches dandischema.exceptions at all. It catches pydantic.ValidationError directly and calls .errors() itself in dandi/files/bases.py (around lines 222 and 809-810), entirely independent of dandischema's exception wrapper. Unaffected by this change.
  • dandi-archive's dandiapi/api/services/metadata/__init__.py (_encode_pydantic_error, lines 28-49) only reads error.errors as a list of dicts with loc/msg keys. That is exactly the shape ve.errors() already produces, so no behavioral change is needed there.
  • dandi-schema's own tests (dandischema/tests/test_metadata.py, e.g. lines 98 and 319) index into .errors the same way, and the one test with different expectations, test_missing_ok, is removed by Remove the missing_ok parameter from metadata.validate() #422 anyway.

No cross-repo breakage is expected from this change.

Follow-up in dandi-archive

Once this change ships, dandi-archive's _encode_pydantic_error parameter (dandiapi/api/services/metadata/__init__.py:28) can be annotated as pydantic_core.ErrorDetails instead of an untyped/implicit dict. This has to be a follow-up PR in that repo rather than part of this one, since it lives outside dandi-schema.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions