Skip to content

Keep converter outputs aligned with root schema - #306

Open
LiangRuoShui wants to merge 1 commit into
apache:mainfrom
LiangRuoShui:fix/schema-valid-converter-output
Open

Keep converter outputs aligned with root schema#306
LiangRuoShui wants to merge 1 commit into
apache:mainfrom
LiangRuoShui:fix/schema-valid-converter-output

Conversation

@LiangRuoShui

Copy link
Copy Markdown

Why

The published core JSON Schema only allows version and semantic_model at the document root and sets additionalProperties: false. dbt and Honeydew could still emit root-level advertisement fields (dialects / vendors), and the Python OSIDocument model accepted those fields, which allowed generated OSI documents to fail the official validator.

What changed

  • Removed root-level dialects emission from the dbt MSI-to-OSI converter.
  • Removed root-level vendors emission from the Honeydew-to-OSI converter.
  • Made Honeydew reject root-level advertisement fields such as vendors and dialects on OSI input.
  • Made the Python OSIDocument model reject schema-invalid root extra fields.
  • Updated dbt snapshots to reflect schema-valid root output.

Tests added/updated

  • Added dbt regression coverage asserting serialized OSI output omits root dialects and vendors while preserving expression-level dialects.
  • Added Honeydew regression coverage asserting generated OSI output omits root advertisement fields.
  • Added Honeydew input validation coverage asserting root vendors / dialects are rejected.
  • Added Python model coverage asserting root dialects is rejected.

Verification

  • uv run validation/validate.py examples/tpcds_semantic_model.yaml
  • cd python && uv run --with pytest pytest -q
  • cd converters/dbt && uv run pytest -q
  • cd converters/honeydew && uv run pytest -q

@LiangRuoShui

Copy link
Copy Markdown
Author

Gentle ping on #306, #307, and #308.

I noticed the workflows for #306 and #308 are waiting for maintainer approval (action_required). Could someone approve the CI runs when convenient?

For review order, #307 and #308 are related:

#306 is independent and keeps converter/Python output aligned with the published root schema.

Happy to adjust or split further if the direction looks off.

@khush-bhatia

Copy link
Copy Markdown
Contributor

Hey @LiangRuoShui Could you please resolve the merge conflicts because of the rename ?

@khush-bhatia
khush-bhatia self-requested a review August 28, 2026 23:20
@LiangRuoShui
LiangRuoShui force-pushed the fix/schema-valid-converter-output branch from 08e8141 to ad69857 Compare August 29, 2026 05:22
@LiangRuoShui

Copy link
Copy Markdown
Author

Thanks for the reminder! I have rebased this PR on the latest main and resolved the merge conflicts caused by the rename. I also ran the relevant local tests to make sure the changes still behave as expected. Please let me know if there is anything else you would like me to adjust.

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.

2 participants