Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing fields in constraints model for manifest v12 #109

Open
airibarne opened this issue Aug 28, 2024 · 5 comments
Open

Missing fields in constraints model for manifest v12 #109

airibarne opened this issue Aug 28, 2024 · 5 comments

Comments

@airibarne
Copy link

Hi @yu-iskw! Firstly, thank you for your contribution, excellent and very needed package within the DBT ecosystem 😄

We are building a tool in top of the dbt-artifacts-parser, and we've found the following: while node column constraints schema specification in the DBT Manifest v12 has to and to_columns fields, the current Pydantic parser interface for v12 does not have them, while has extra=forbid setting. This makes the parser to fail parsing v12 manifests obtained from projects having models with constraints.

Here the v12 schema specification: https://schemas.getdbt.com/dbt/manifest/v12/index.html#nodes_additionalProperties_anyOf_i0_columns_additionalProperties_constraints

Here the class representation: https://github.com/yu-iskw/dbt-artifacts-parser/blob/main/dbt_artifacts_parser/parsers/manifest/manifest_v12.py#L128-L136

Would be happy to make a PR after any discussion you might want to have.

@yu-iskw
Copy link
Owner

yu-iskw commented Aug 28, 2024

@airibarne thank you for raising the issue. I didn't catch up with the changes on manifest v12. It seems that the dbt version in manivest v12 is 1.9.0a1 at the time of writing this. According to my research, to and to_columns were added after changing the version in the JSON schema. I feel it might be good to wait until the stable version of dbt 1.9 is released for the reliability, though I don't think we will have degraded changes in the schema until then. What do you think?

https://github.com/dbt-labs/dbt-core/blob/98fddcf54ff985fc97e0aafccb357f3d93d3fdbd/schemas/dbt/manifest/v12.json#L16

@psygnoser
Copy link

I've already raised this month ago here #99

@airibarne
Copy link
Author

airibarne commented Aug 29, 2024

Hi there! Oh @psygnoser, sorry I did not find the issue at first glance. It is indeed the same I'm reporting here.

@yu-iskw, it's no blocker for us to wait until the release of stable 1.9, so we have no problem with that. However, we've encountered this issue trying to parse a manifest generated with 1.8, so there might be other users of this library encountering this error. Just let me know if there's anything we can do about it 😄

@yu-iskw
Copy link
Owner

yu-iskw commented Aug 30, 2024

Thought the update doesn't solve this issue, I am updating manifest_v12.py with the JSON schema at dbt-core 1.8.6.

#110

@pgoslatara
Copy link

@yu-iskw Any thoughts on a stable way forward on this? I just opened #112 because one of my scripts failed due to the presence of a new field. Feels like we should permit these new fields, otherwise we will experience failures due to the presence of fields we do not use.

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

No branches or pull requests

4 participants