From 73c7755f2e8bd210a3da0f3b3911d43eb4a8161d Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Wed, 28 Feb 2024 18:20:58 -0600 Subject: [PATCH 1/3] add pre_serialization method and tests --- core/dbt/artifacts/resources/base.py | 17 +- .../artifacts/resources/test_serialization.py | 166 ++++++++++++++++++ 2 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 tests/unit/artifacts/resources/test_serialization.py diff --git a/core/dbt/artifacts/resources/base.py b/core/dbt/artifacts/resources/base.py index dd66aa97d72..9199b6e01fe 100644 --- a/core/dbt/artifacts/resources/base.py +++ b/core/dbt/artifacts/resources/base.py @@ -1,4 +1,4 @@ -from dataclasses import dataclass +from dataclasses import dataclass, fields from dbt_common.dataclass_schema import dbtClassMixin from typing import List, Optional import hashlib @@ -15,6 +15,21 @@ class BaseResource(dbtClassMixin): original_file_path: str unique_id: str + @classmethod + def __pre_deserialize__(cls, data): + data = super().__pre_deserialize__(data) + + # Support deserializing additional fields in BaseResource for forward + # compatibility within a major version + for f in fields(cls): + # missing fields with defaults are acceptable + if f.name not in data and f.default: + data[f.name] = f.default + # missing optional fields are acceptable + elif f.name not in data and f.init is False: + data[f.name] = None + return data + @dataclass class GraphResource(BaseResource): diff --git a/tests/unit/artifacts/resources/test_serialization.py b/tests/unit/artifacts/resources/test_serialization.py new file mode 100644 index 00000000000..9e793393137 --- /dev/null +++ b/tests/unit/artifacts/resources/test_serialization.py @@ -0,0 +1,166 @@ +import pytest +from dataclasses import dataclass +from typing import Optional +from mashumaro.exceptions import MissingField + +from dbt.artifacts.resources.base import BaseResource +from dbt.artifacts.resources.types import NodeType + + +# Test that a (mocked) new minor version of a BaseResource (serialized with a value for +# a new optional field) can be deserialized successfully. e.g. something like +# PreviousBaseResource.from_dict(CurrentBaseResource(...).to_dict()) +@dataclass +class SlimClass(BaseResource): + my_str: str + + +@dataclass +class OptionalFieldClass(BaseResource): + my_str: str + optional_field: Optional[str] = None + + +@dataclass +class RequiredFieldClass(BaseResource): + my_str: str + new_field: str + + +# Test that a new minor version of a BaseResource serialized with a +# field that is now optional, but did not previously exist can be +# deserialized successfully. +def test_adding_optional_field(): + current_instance = OptionalFieldClass( + name="test", + resource_type=NodeType.Macro, + package_name="awsome_package", + path="my_path", + original_file_path="my_file_path", + unique_id="abc", + my_str="test", + optional_field="test", # new optional field + ) + + current_instance_dict = current_instance.to_dict() + expected_current_dict = { + "name": "test", + "resource_type": "macro", + "package_name": "awsome_package", + "path": "my_path", + "original_file_path": "my_file_path", + "unique_id": "abc", + "my_str": "test", + "optional_field": "test", + } + assert current_instance_dict == expected_current_dict + + expected_slim_instance = SlimClass( + name="test", + resource_type=NodeType.Macro, + package_name="awsome_package", + path="my_path", + original_file_path="my_file_path", + unique_id="abc", + my_str="test", + ) + slim_instance = SlimClass.from_dict(current_instance_dict) + assert slim_instance == expected_slim_instance + + +# Test that a new minor version of a BaseResource serialized without a +# field that was previously optional can be deserialized successfully. +def test_missing_optional_field(): + current_instance = SlimClass( + name="test", + resource_type=NodeType.Macro, + package_name="awsome_package", + path="my_path", + original_file_path="my_file_path", + unique_id="abc", + my_str="test", + # optional_field="test" -> puposely excluded + ) + current_instance_dict = current_instance.to_dict() + expected_current_dict = { + "name": "test", + "resource_type": "macro", + "package_name": "awsome_package", + "path": "my_path", + "original_file_path": "my_file_path", + "unique_id": "abc", + "my_str": "test", + } + assert current_instance_dict == expected_current_dict + + expected_optional_field_instance = OptionalFieldClass( + name="test", + resource_type=NodeType.Macro, + package_name="awsome_package", + path="my_path", + original_file_path="my_file_path", + unique_id="abc", + my_str="test", + optional_field=None, + ) + slim_instance = OptionalFieldClass.from_dict(current_instance_dict) + assert slim_instance == expected_optional_field_instance + + +# Test that a new minor version of a BaseResource serialized with a +# new field without a default, but did not previously exist can be +# deserialized successfully +def test_adding_required_field(): + current_instance = RequiredFieldClass( + name="test", + resource_type=NodeType.Macro, + package_name="awsome_package", + path="my_path", + original_file_path="my_file_path", + unique_id="abc", + my_str="test", + new_field="test", # new required field + ) + + current_instance_dict = current_instance.to_dict() + expected_current_dict = { + "name": "test", + "resource_type": "macro", + "package_name": "awsome_package", + "path": "my_path", + "original_file_path": "my_file_path", + "unique_id": "abc", + "my_str": "test", + "new_field": "test", + } + assert current_instance_dict == expected_current_dict + + expected_slim_instance = SlimClass( + name="test", + resource_type=NodeType.Macro, + package_name="awsome_package", + path="my_path", + original_file_path="my_file_path", + unique_id="abc", + my_str="test", + ) + slim_instance = SlimClass.from_dict(current_instance_dict) + assert slim_instance == expected_slim_instance + + +# Test that a new minor version of a BaseResource serialized without a +# field with no default cannot be deserialized successfully. We don't +# want to allow removing required fields. Expect error. +def test_removing_required_field(): + current_instance = SlimClass( + name="test", + resource_type=NodeType.Macro, + package_name="awsome_package", + path="my_path", + original_file_path="my_file_path", + unique_id="abc", + my_str="test", + ) + expecter_err = 'Field "new_field" of type str is missing in RequiredFieldClass instance' + with pytest.raises(MissingField, match=expecter_err): + RequiredFieldClass.from_dict(current_instance.to_dict()) From d53d216d4a1e4824b5ae23fb1f82fa872ffccad7 Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Fri, 1 Mar 2024 09:18:12 -0600 Subject: [PATCH 2/3] changelog --- .changes/unreleased/Features-20240301-091804.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Features-20240301-091804.yaml diff --git a/.changes/unreleased/Features-20240301-091804.yaml b/.changes/unreleased/Features-20240301-091804.yaml new file mode 100644 index 00000000000..98e7e987fdd --- /dev/null +++ b/.changes/unreleased/Features-20240301-091804.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Allow adding new fields and removing optional fields to BaseResource subclasses +time: 2024-03-01T09:18:04.696662-06:00 +custom: + Author: emmyoop + Issue: "9615" From 77210358e1214bbde72976e4f09d22fb22d62d5e Mon Sep 17 00:00:00 2001 From: Emily Rockman Date: Fri, 1 Mar 2024 10:13:56 -0600 Subject: [PATCH 3/3] Update Features-20240301-091804.yaml --- .changes/unreleased/Features-20240301-091804.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changes/unreleased/Features-20240301-091804.yaml b/.changes/unreleased/Features-20240301-091804.yaml index 98e7e987fdd..b1eecefb395 100644 --- a/.changes/unreleased/Features-20240301-091804.yaml +++ b/.changes/unreleased/Features-20240301-091804.yaml @@ -1,5 +1,5 @@ kind: Features -body: Allow adding new fields and removing optional fields to BaseResource subclasses +body: Allow adding new fields and removing optional fields to BaseResource subclasses without creating new versions of artifacts time: 2024-03-01T09:18:04.696662-06:00 custom: Author: emmyoop