yu-iskw / dbt-artifacts-parser

A dbt artifacts parser in python
https://pypi.org/project/dbt-artifacts-parser/
Apache License 2.0
66 stars 16 forks source link

Missing fields in constraints model for manifest v12 #109

Open airibarne opened 1 month ago

airibarne commented 1 month ago

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 commented 1 month ago

@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 commented 4 weeks ago

I've already raised this month ago here https://github.com/yu-iskw/dbt-artifacts-parser/issues/99

airibarne commented 4 weeks ago

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 :smile:

yu-iskw commented 4 weeks ago

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

https://github.com/yu-iskw/dbt-artifacts-parser/pull/110

pgoslatara commented 1 week ago

@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.