Skip to content

fix(validation): report schema errors for invalid YAML - #271

Open
mattfaltyn wants to merge 1 commit into
apache:mainfrom
mattfaltyn:fix/validator-non-mapping-yaml
Open

fix(validation): report schema errors for invalid YAML#271
mattfaltyn wants to merge 1 commit into
apache:mainfrom
mattfaltyn:fix/validator-non-mapping-yaml

Conversation

@mattfaltyn

@mattfaltyn mattfaltyn commented Jul 26, 2026

Copy link
Copy Markdown

Summary

JSON Schema validation already detects invalid YAML shapes, but the CLI continued into semantic checks that assume schema-valid mappings and lists. Those checks could raise AttributeError before the existing reporter displayed the schema errors.

This change:

  • reports the schema errors for empty, sequence, scalar, and malformed nested YAML;
  • runs semantic checks only after schema validation succeeds; and
  • adds focused CLI regression coverage using the Python standard library.

The specification, CLI arguments, and dependency set are unchanged.

Related Issues

Closes #270

Testing

Focused validation

uv run --with 'jsonschema>=4.26.0' --with 'pyyaml>=6.0.3' \
  --with 'sqlglot>=30.12.0' python -m unittest validation.test_validate -v
uv run validation/validate.py /dev/null --schema core-spec/osi-schema.json
uv run validation/validate.py examples/tpcds_semantic_model.yaml \
  --schema core-spec/osi-schema.json
python -m py_compile validation/validate.py validation/test_validate.py
git diff --check

Results:

  • The regression test passes all four input cases.
  • Empty YAML exits 1 with the expected schema error and no traceback.
  • The valid TPC-DS example passes.
  • Compilation, Ruff, and diff checks pass.

Repository-wide validation

  • All Python binding and converter suites pass except the Omni baseline noted below.
  • CLI go build, go vet, and go test pass.
  • Polaris and Salesforce Maven verify builds pass.
  • The Omni suite has two pre-existing TPC-DS fixture failures, reproduced unchanged on the PR base (8c410f9); this PR does not modify Omni or the shared example.
  • No current GitHub Actions workflow is path-matched to validation/**, so this PR has no remote checks.

Checklist

Validation

  • New validation cases are covered by tests

Documentation

  • Existing documentation was reviewed; no update is needed because the validator interface and documented contract are unchanged

Tests

  • All tests relevant to this change pass locally
  • New functionality is covered by tests

Compliance

  • ASF license headers are present on all new source files
  • No third-party dependencies are added

Signed-off-by: Matt Faltyn <faltyn.matthew@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validator crashes instead of reporting errors for invalid YAML structures

1 participant