Uh oh!
There was an error while loading. Please reload this page.
fix(dbt): serialize osi-to-msi manifest with pydantic v1 .json() - #331
Open
ntoxlut wants to merge 1 commit into
Open
fix(dbt): serialize osi-to-msi manifest with pydantic v1 .json()#331ntoxlut wants to merge 1 commit into
ntoxlut wants to merge 1 commit into
Conversation
The osi-to-msi CLI called .model_dump_json() on the converter output, but PydanticSemanticManifest (from metricflow_semantic_interfaces) subclasses pydantic.v1.BaseModel, whose serializer is .json(). As a result osi-to-msi raised AttributeError on every input, including the repository's own examples/tpcds_semantic_model.yaml. Switch to .json(by_alias=True, exclude_none=True, indent=2), which produces identical serialization options on a pydantic v1 model, and add an end-to-end CLI test that drives osi-to-msi and asserts the output is a valid semantic manifest. No test covered the CLI before, so CI did not catch the crash. Fixesapache#296 Signed-off-by: Ntokozo Luthuli <ntokozomshibe@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ossie-dbt osi-to-msifails on every input, including the repository's ownexamples/tpcds_semantic_model.yaml, with:PydanticSemanticManifest(frommetricflow_semantic_interfaces) subclassespydantic.v1.BaseModel, whose JSON serializer is.json(), not the pydantic v2.model_dump_json(). The conversion itself works — only the CLI's write stepwas calling a v2-only method on a v1 object.
Fix
cli.py: use.json(by_alias=True, exclude_none=True, indent=2), which takesthe same options on a pydantic v1 model and produces valid output.
tests/test_cli.py— an end-to-end test that drivesosi-to-msiandasserts a valid semantic manifest is written. No test covered the CLI before,
which is why CI stayed green through the crash.
Verification
The new test fails without the fix (
AttributeError) and passes with it.Related Issues
Fixes#296
Checklist
Specification
N/A — bug fix only, no spec change.
Ontology
N/A — no ontology change.
Converters
Validation
N/A — no validation change.
Documentation
N/A — no user-facing behavior change beyond the command no longer crashing.
Examples
N/A — no new spec constructs or converter support.
Tests
pytest/ CI green)Compliance