From cd64b899659f7e490a4ffeb80ead84a3898f42f2 Mon Sep 17 00:00:00 2001 From: Chenyu Li Date: Fri, 13 Dec 2024 12:17:30 -0800 Subject: [PATCH] allow additional for manifest object --- .../Under the Hood-20241211-160216.yaml | 2 +- core/dbt/artifacts/resources/v1/model.py | 9 +------ .../resources/v1/source_definition.py | 2 +- core/dbt/contracts/graph/manifest.py | 27 +++++++++++++++++++ core/dbt/contracts/graph/nodes.py | 6 ----- tests/unit/contracts/graph/test_nodes.py | 2 +- .../unit/contracts/graph/test_nodes_parsed.py | 3 ++- tests/unit/utils/__init__.py | 2 ++ 8 files changed, 35 insertions(+), 18 deletions(-) diff --git a/.changes/unreleased/Under the Hood-20241211-160216.yaml b/.changes/unreleased/Under the Hood-20241211-160216.yaml index 8192dc9b303..26dfc4f05f4 100644 --- a/.changes/unreleased/Under the Hood-20241211-160216.yaml +++ b/.changes/unreleased/Under the Hood-20241211-160216.yaml @@ -1,5 +1,5 @@ kind: Under the Hood -body: Allow additional property on Model and SourceDefinition to avoid artifact read +body: Allow additional property on manifest when loading problem time: 2024-12-11T16:02:16.551106-08:00 custom: diff --git a/core/dbt/artifacts/resources/v1/model.py b/core/dbt/artifacts/resources/v1/model.py index adcbf069464..9c43970f488 100644 --- a/core/dbt/artifacts/resources/v1/model.py +++ b/core/dbt/artifacts/resources/v1/model.py @@ -10,7 +10,6 @@ ) from dbt.artifacts.resources.v1.config import NodeConfig from dbt_common.contracts.config.base import MergeBehavior -from dbt_common.contracts.config.properties import AdditionalPropertiesAllowed from dbt_common.contracts.constraints import ModelLevelConstraint from dbt_common.dataclass_schema import dbtClassMixin @@ -36,7 +35,7 @@ class TimeSpine(dbtClassMixin): @dataclass -class Model(AdditionalPropertiesAllowed, CompiledResource): +class Model(CompiledResource): resource_type: Literal[NodeType.Model] access: AccessType = AccessType.Protected config: ModelConfig = field(default_factory=ModelConfig) @@ -52,10 +51,4 @@ def __post_serialize__(self, dct: Dict, context: Optional[Dict] = None): dct = super().__post_serialize__(dct, context) if context and context.get("artifact") and "defer_relation" in dct: del dct["defer_relation"] - - # delete extra keys that should't be serialized - if self.extra: - for key, _ in self.extra.items(): - if key in dct: - del dct[key] return dct diff --git a/core/dbt/artifacts/resources/v1/source_definition.py b/core/dbt/artifacts/resources/v1/source_definition.py index 2dbe1d2bb1d..9044307563e 100644 --- a/core/dbt/artifacts/resources/v1/source_definition.py +++ b/core/dbt/artifacts/resources/v1/source_definition.py @@ -56,7 +56,7 @@ class ParsedSourceMandatory(GraphResource, HasRelationMetadata): @dataclass -class SourceDefinition(AdditionalPropertiesAllowed, ParsedSourceMandatory): +class SourceDefinition(ParsedSourceMandatory): quoting: Quoting = field(default_factory=Quoting) loaded_at_field: Optional[str] = None freshness: Optional[FreshnessThreshold] = None diff --git a/core/dbt/contracts/graph/manifest.py b/core/dbt/contracts/graph/manifest.py index 0168abe8fd8..e09c3899844 100644 --- a/core/dbt/contracts/graph/manifest.py +++ b/core/dbt/contracts/graph/manifest.py @@ -20,6 +20,8 @@ Union, ) +import jsonschema +from jsonschema import ValidationError from typing_extensions import Protocol import dbt_common.exceptions @@ -936,6 +938,31 @@ class Manifest(MacroMethods, dbtClassMixin): metadata={"serialize": lambda x: None, "deserialize": lambda x: None}, ) + # This and the following methods can be refacong into a mixin + # and we should do so next time we do it for another artifact. + @classmethod + def validate(cls, data: Any) -> None: + json_schema = cls.json_schema() + cls._allow_additional_recursive(cls, json_schema) + validator = jsonschema.Draft7Validator(json_schema) + error = next(iter(validator.iter_errors(data)), None) + if error is not None: + raise ValidationError.create_from(error) from error + + @staticmethod + def _allow_additional_recursive(cls, dct: Dict[str, Any]): + if ( + "proproties" in dct + and "additionalProperties" in dct + and dct["additionalProperties"] is False + ): + dct["additionalProperties"] = True + for k, v in dct.items(): + if isinstance(v, dict): + dct[k] = cls._allow_additional_recursive(dct.get(k, {})) + if isinstance(v, list): + dct[k] = [cls._allow_additional_recursive(i) for i in v] + def __pre_serialize__(self, context: Optional[Dict] = None): # serialization won't work with anything except an empty source_patches because # tuple keys are not supported, so ensure it's empty diff --git a/core/dbt/contracts/graph/nodes.py b/core/dbt/contracts/graph/nodes.py index e919cb9fff1..4bb70db5d9c 100644 --- a/core/dbt/contracts/graph/nodes.py +++ b/core/dbt/contracts/graph/nodes.py @@ -1371,12 +1371,6 @@ def search_name(self): def group(self): return None - def __post_serialize__(self, dct: Dict, context: Optional[Dict] = None): - dct = super().__post_serialize__(dct, context) - if "_event_status" in dct: - del dct["_event_status"] - return dct - # ==================================== # Exposure node diff --git a/tests/unit/contracts/graph/test_nodes.py b/tests/unit/contracts/graph/test_nodes.py index 75184fb72cc..304632ec7d5 100644 --- a/tests/unit/contracts/graph/test_nodes.py +++ b/tests/unit/contracts/graph/test_nodes.py @@ -240,7 +240,7 @@ def test_extra_fields_model_okay(minimal_uncompiled_dict): extra = minimal_uncompiled_dict extra["notvalid"] = "nope" # Model still load fine with extra fields - assert ModelNode.from_dict(extra)._extra == {"notvalid": "nope"} + ModelNode.from_dict(extra) def test_invalid_bad_type_model(minimal_uncompiled_dict): diff --git a/tests/unit/contracts/graph/test_nodes_parsed.py b/tests/unit/contracts/graph/test_nodes_parsed.py index a8b1c917f6c..bf8fcfbea5a 100644 --- a/tests/unit/contracts/graph/test_nodes_parsed.py +++ b/tests/unit/contracts/graph/test_nodes_parsed.py @@ -1950,6 +1950,7 @@ def test_basic_source_definition( node = basic_parsed_source_definition_object node_dict = basic_parsed_source_definition_dict minimum = minimum_parsed_source_definition_dict + assert_symmetric(node.to_resource(), node_dict, SourceDefinitionResource) assert node.is_ephemeral is False @@ -1964,7 +1965,7 @@ def test_extra_fields_source_definition_okay(minimum_parsed_source_definition_di extra = minimum_parsed_source_definition_dict extra["notvalid"] = "nope" # Model still load fine with extra fields - assert SourceDefinition.from_dict(extra)._extra == {"notvalid": "nope"} + SourceDefinition.from_dict(extra) def test_invalid_missing(minimum_parsed_source_definition_dict): diff --git a/tests/unit/utils/__init__.py b/tests/unit/utils/__init__.py index 76b6653d30a..ec9cb57595d 100644 --- a/tests/unit/utils/__init__.py +++ b/tests/unit/utils/__init__.py @@ -197,9 +197,11 @@ def assert_from_dict(obj, dct, cls=None): cls.validate(dct) obj_from_dict = cls.from_dict(dct) + if hasattr(obj, "created_at"): obj_from_dict.created_at = 1 obj.created_at = 1 + assert obj_from_dict == obj