Skip to content

Commit

Permalink
add tests and an error to make sure schema update are safe
Browse files Browse the repository at this point in the history
  • Loading branch information
ChenyuLInx committed Dec 13, 2024
1 parent 7df04b0 commit fda1eb9
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 4 deletions.
10 changes: 10 additions & 0 deletions core/dbt/artifacts/schemas/manifest/v12/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,3 +180,13 @@ def upgrade_schema_version(cls, data):
if manifest_schema_version < cls.dbt_schema_version.version:
data = upgrade_manifest_json(data, manifest_schema_version)
return cls.from_dict(data)

@classmethod
def validate(cls, _):
# When dbt try to load an artifact with additional optional fields
# that are not present in the schema, from_dict will work fine.
# As long as validate is not called, the schema will not be enforced.
# This is intentional, as it allows for safer schema upgrades.
raise RuntimeError(
"The WritableManifest should never be validated directly to allow for schema upgrades."
)
16 changes: 16 additions & 0 deletions tests/functional/artifacts/test_artifacts.py
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,22 @@ def test_run_and_generate(self, project, manifest_schema_path, run_results_schem
> 0
)

# Test artifact with additional fields load fine
def test_load_artifact(self, project, manifest_schema_path, run_results_schema_path):
catcher = EventCatcher(ArtifactWritten)
results = run_dbt(args=["compile"], callbacks=[catcher.catch])
assert len(results) == 7
manifest_dct = get_artifact(os.path.join(project.project_root, "target", "manifest.json"))
# add a field that is not in the schema
for _, node in manifest_dct["nodes"].items():
node["something_else"] = "something_else"
# load the manifest with the additional field
loaded_manifest = WritableManifest.from_dict(manifest_dct)

# successfully loaded the manifest with the additional field, but the field should not be present
for _, node in loaded_manifest.nodes.items():
assert not hasattr(node, "something_else")


class TestVerifyArtifactsReferences(BaseVerifyProject):
@pytest.fixture(scope="class")
Expand Down
10 changes: 6 additions & 4 deletions tests/unit/contracts/graph/test_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,10 +236,12 @@ def test_basic_compiled_model(basic_compiled_dict, basic_compiled_model):
assert node.is_ephemeral is False


def test_invalid_extra_fields_model(minimal_uncompiled_dict):
bad_extra = minimal_uncompiled_dict
bad_extra["notvalid"] = "nope"
assert_fails_validation(bad_extra, ModelNode)
def test_extra_fields_model_okay(minimal_uncompiled_dict):
extra = minimal_uncompiled_dict
extra["notvalid"] = "nope"
# Model still load fine with extra fields
loaded_model = ModelNode.from_dict(extra)
assert not hasattr(loaded_model, "notvalid")


def test_invalid_bad_type_model(minimal_uncompiled_dict):
Expand Down
8 changes: 8 additions & 0 deletions tests/unit/contracts/graph/test_nodes_parsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -1961,6 +1961,14 @@ def test_basic_source_definition(
pickle.loads(pickle.dumps(node))


def test_extra_fields_source_definition_okay(minimum_parsed_source_definition_dict):
extra = minimum_parsed_source_definition_dict
extra["notvalid"] = "nope"
# Model still load fine with extra fields
loaded_source = SourceDefinition.from_dict(extra)
assert not hasattr(loaded_source, "notvalid")


def test_invalid_missing(minimum_parsed_source_definition_dict):
bad_missing_name = minimum_parsed_source_definition_dict
del bad_missing_name["name"]
Expand Down

0 comments on commit fda1eb9

Please sign in to comment.