Add an option to be more permissive with extra fields for manifest parsing

[aballiet]

Add an option to be more permissive with extra fields for manifest parsing

[MiConnell]

I agree, I was just struggling with this issue. I opened a PR (#104) to update the default to allow from forbid

[aballiet]

I understand the philosophy. I wnat to clarify two things about extra fields.

1. Should we allow to add a new fields in anywhere at any level?
2. I feel this can be controlled by the JSON schemas of dbt artifacts.

First, I want to know the philosophy is applied to any fields at any level? For instance, it sounds natural to allow extra fields in meta properties. But, I am personally upset to allow extra fields at any level.

Second, we can control to allow additional fields by the JSON schema of dbt artifacts. For instance, the columns property in a node has the additionalProperties: true property. So, the generated class has extra fields. I feel it would be good to clearly define where we can use additional fields in dbt artifacts. We can technically customize the generatec classes from the JSON schema of dbt artifacts. But, it would be a bit difficult to settle what we can customize in the package. I want to keep generated classes from the schemas as original as possible, because it is a bit hassle to manage the additional changes in generating code.

I totally understand that it is quite difficult to change the JSON schema of dbt artifacts of the older versions in the dbt-core repository. But, I want to change the JSON schemas, not generatec class, in the repository.

• https://github.com/yu-iskw/dbt-artifacts-parser/blob/7ac8c85474cb1c19b1ef75ee422e81c701eafb99/dbt_artifacts_parser/resources/manifest/manifest_v12.json#L572C19-L572C48

• dbt-artifacts-parser/dbt_artifacts_parser/parsers/manifest/manifest_v12.py

  Line 141 in 7ac8c85

  extra='allow',

Agree that the best is to handle extra fields policy in JSON Schema directly but currently it's not the case.

IMO for now as a temporary fix to make your package relevant for most users, we should allow at any-level to make sure parsing is handled.

You could think about an extra parameter for the function

dbt-artifacts-parser/dbt_artifacts_parser/parser.py

Line 74 in 7ac8c85

def parse_manifest(

like allow_extra_fields default to True

[yu-iskw]

I understand the philosophy. I wnat to clarify two things about extra fields.

1. Should we allow to add a new fields in anywhere at any level?
2. I feel this can be controlled by the JSON schemas of dbt artifacts.

First, I want to know the philosophy is applied to any fields at any level? For instance, it sounds natural to allow extra fields in meta properties. But, I am personally upset to allow extra fields at any level.

Second, we can control to allow additional fields by the JSON schema of dbt artifacts. For instance, the columns property in a node has the additionalProperties: true property. So, the generated class has extra fields. I feel it would be good to clearly define where we can use additional fields in dbt artifacts. We can technically customize the generatec classes from the JSON schema of dbt artifacts. But, it would be a bit difficult to settle what we can customize in the package. I want to keep generated classes from the schemas as original as possible, because it is a bit hassle to manage the additional changes in generating code.

I totally understand that it is quite difficult to change the JSON schema of dbt artifacts of the older versions in the dbt-core repository. But, I want to change the JSON schemas, not generatec class, in the repository.

• https://github.com/yu-iskw/dbt-artifacts-parser/blob/7ac8c85474cb1c19b1ef75ee422e81c701eafb99/dbt_artifacts_parser/resources/manifest/manifest_v12.json#L572C19-L572C48

• dbt-artifacts-parser/dbt_artifacts_parser/parsers/manifest/manifest_v12.py

  Line 141 in 7ac8c85

  extra='allow',

[yu-iskw]

but currently it's not the case.

Can you elaborate on it? I am still upset to apply a temporary solution due to the two reasons:

1. it is hard to manage manual changes to generated files, because we have to apply them every time when we regenerate classes.
2. The official schemas explicitely declare "additionalProperties": false. I want to admire the official declarations as much as possible. We use the datamodel-code-generator package to generatge classes from the JSON schemas. It doesn't specify the model configuration about extra fields, if the "additionalProperties" properties aren't set.
   • e.g. manifest v12: https://github.com/dbt-labs/dbt-core/blob/main/schemas/dbt/manifest/v12.json#L559

I totally understand some users want the temporary changes for extra fields. Meanwhile, we have to beare in mind other users to validate their inputs with the strict properties.

[aballiet]

@aballiet BTW, can you tell me what kind of extra fields you want to add? I want to enchance the resolution about what some users want to do with the package. It would be good to give feedback not only to the repository, but also the dbt community, as we can also change the official JSON schemas in the dbt-core repository. That might be helpful for other dbt users as well.

I don't want to add any extra fields, just wanted to make your current code working on dbt cloud manifest haha

You can check doing your usual commands:

curl https://schemas.getdbt.com/dbt/manifest/v12.json -o json_schema_v12.json
datamodel-codegen  --input json_schema_v12.json --input-file-type jsonschema --output model.py

[jtcohen6]

+1 from the dbt team. This permissiveness would be better aligned with our philosophy that adding a new field (with a default) is a non-breaking change, and therefore does not require a major-version bump to artifacts: https://docs.getdbt.com/blog/latest-dbt-stability#stability-for-metadata-artifacts

From the dbt-core artifacts README

Non-breaking changes
Freely make incremental, non-breaking changes in-place to the latest major version of any artifact (minor or patch bumps). The only changes that are fully forward and backward compatible are:

• Adding a new field with a default
• Deleting a field with a default

...

Note: Although jsonschema validation using the schemas in schemas.getdbt.com is not encouraged or formally supported, jsonschema validation should still continue to work once the schemas are updated because they are forward-compatible and can therefore be used to validate previous minor versions of the schema.

[aballiet]

but currently it's not the case.

Can you elaborate on it? I am still upset to apply a temporary solution due to the two reasons:

1. it is hard to manage manual changes to generated files, because we have to apply them every time when we regenerate classes.
2. The official schemas explicitely declare "additionalProperties": false. I want to admire the official declarations as much as possible. We use the datamodel-code-generator package to generatge classes from the JSON schema. It doesn't specify the model configuration about extra fields, if the "additionalProperties" properties are set.
   • e.g. manifest v12: https://github.com/dbt-labs/dbt-core/blob/main/schemas/dbt/manifest/v12.json#L559

I totally understand some users want to the temporary changes for extra fields. Meanwhile, we have to beare in mind other users to validate their inputs with the strict properties.

Totally agree with you and I am not from dbt labs.

Maybe @jtcohen6 could answer this one on why dbt-labs does not write their jsonschema with this in mind as agree with you it's the purpose of schemas.

Will also ask their dbt cloud support...

[yu-iskw]

@aballiet BTW, can you tell me what kind of extra fields you want to add? I want to enchance the resolution about what some users want to do with the package. It would be good to give feedback not only to the repository, but also the dbt community, as we can also change the official JSON schemas in the dbt-core repository. That might be helpful for other dbt users as well.

[yu-iskw]

In that case, how about validating your artifacts with the official JSON schemas using something like check-jsonschema. By doing this, we can break down the issue. If you see any issues just on the validation, I would be great to give feedback to the dbt community and dbt Labs.

https://github.com/python-jsonschema/check-jsonschema

[aballiet]

NVM they fixed on DBT Cloud side, now manifest parser works as expected 👍. Closing the issue