Skip to content

Commit

Permalink
PR review. DeferRelation conforms to RelationConfig protocol
Browse files Browse the repository at this point in the history
  • Loading branch information
jtcohen6 committed May 1, 2024
1 parent 6b63e31 commit 7854bfa
Show file tree
Hide file tree
Showing 8 changed files with 41 additions and 25 deletions.
9 changes: 9 additions & 0 deletions core/dbt/artifacts/resources/v1/components.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import time
from dataclasses import dataclass, field
from dbt.artifacts.resources.base import GraphResource, FileHash, Docs
from dbt.artifacts.resources.types import NodeType
from dbt.artifacts.resources.v1.config import NodeConfig
from dbt_common.dataclass_schema import dbtClassMixin, ExtensibleDbtClassMixin
from dbt_common.contracts.config.properties import AdditionalPropertiesMixin
Expand Down Expand Up @@ -154,6 +155,14 @@ def quoting_dict(self) -> Dict[str, bool]:
class DeferRelation(HasRelationMetadata):
alias: str
relation_name: Optional[str]
# The rest of these fields match RelationConfig protocol exactly
resource_type: NodeType
name: str
description: str
compiled_code: Optional[str]
meta: Dict[str, Any]
tags: List[str]
config: Optional[NodeConfig]

@property
def identifier(self):
Expand Down
2 changes: 2 additions & 0 deletions core/dbt/artifacts/schemas/manifest/v12/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,4 +186,6 @@ def __post_serialize__(self, dct):
for unique_id, node in dct["nodes"].items():
if "config_call_dict" in node:
del node["config_call_dict"]
if "defer_relation" in node:
del node["defer_relation"]
return dct
17 changes: 15 additions & 2 deletions core/dbt/contracts/graph/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
ResultNode,
SavedQuery,
SemanticModel,
SeedNode,
SourceDefinition,
UnpatchedSourceDefinition,
UnitTestDefinition,
Expand All @@ -53,6 +54,7 @@
DeferRelation,
BaseResource,
)
from dbt.artifacts.resources.v1.config import NodeConfig
from dbt.artifacts.schemas.manifest import WritableManifest, ManifestMetadata, UniqueID
from dbt.contracts.files import (
SourceFile,
Expand Down Expand Up @@ -1466,7 +1468,7 @@ def is_invalid_protected_ref(
)

# Called in GraphRunnableTask.before_run, RunTask.before_run, CloneTask.before_run
def merge_from_artifact(self, other: "Manifest", favor_state: bool = False) -> None:
def merge_from_artifact(self, other: "Manifest") -> None:
"""Update this manifest by adding the 'defer_relation' attribute to all nodes
with a counterpart in the stateful manifest used for deferral.
Expand All @@ -1476,8 +1478,19 @@ def merge_from_artifact(self, other: "Manifest", favor_state: bool = False) -> N
for unique_id, node in other.nodes.items():
current = self.nodes.get(unique_id)
if current and node.resource_type in refables and not node.is_ephemeral:
assert isinstance(node.config, NodeConfig) # this makes mypy happy
defer_relation = DeferRelation(
node.database, node.schema, node.alias, node.relation_name
database=node.database,
schema=node.schema,
alias=node.alias,
relation_name=node.relation_name,
resource_type=node.resource_type,
name=node.name,
description=node.description,
compiled_code=(node.compiled_code if not isinstance(node, SeedNode) else None),
meta=node.meta,
tags=node.tags,
config=node.config,
)
self.nodes[unique_id] = replace(current, defer_relation=defer_relation)

Expand Down
5 changes: 5 additions & 0 deletions core/dbt/contracts/graph/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -877,6 +877,11 @@ def language(self):
return "sql"


# @property
# def compiled_code(self):
# return None


# ====================================
# Singular Test node
# ====================================
Expand Down
8 changes: 7 additions & 1 deletion core/dbt/task/clone.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import threading
from typing import AbstractSet, Any, List, Iterable, Set
from typing import AbstractSet, Any, List, Iterable, Set, Optional

from dbt.adapters.base import BaseRelation
from dbt.clients.jinja import MacroGenerator
from dbt.context.providers import generate_runtime_model_context
from dbt.contracts.graph.manifest import Manifest
from dbt.artifacts.schemas.run import RunStatus, RunResult
from dbt_common.dataclass_schema import dbtClassMixin
from dbt_common.exceptions import DbtInternalError, CompilationError
Expand Down Expand Up @@ -93,6 +94,11 @@ class CloneTask(GraphRunnableTask):
def raise_on_first_error(self):
return False

def _get_deferred_manifest(self) -> Optional[Manifest]:
# Unlike other commands, 'clone' always requires a state manifest
# Load previous state, regardless of whether --defer flag has been set
return self._get_previous_state()

def get_model_schemas(self, adapter, selected_uids: Iterable[str]) -> Set[BaseRelation]:
if self.manifest is None:
raise DbtInternalError("manifest was None in get_model_schemas")
Expand Down
9 changes: 1 addition & 8 deletions core/dbt/task/runnable.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,14 +135,7 @@ def defer_to_manifest(self):
raise DbtInternalError(
"Expected to defer to manifest, but there is no runtime manifest to defer from!"
)
self.manifest.merge_from_artifact(
other=deferred_manifest,
favor_state=bool(self.args.favor_state),
)
# We're rewriting the manifest because it's been mutated during merge_from_artifact.
# This is to reflect which nodes had the defer_relation attribute added.
if self.args.write_json:
write_manifest(self.manifest, self.config.project_target_path)
self.manifest.merge_from_artifact(other=deferred_manifest)

def get_graph_queue(self) -> GraphQueue:
selector = self.get_node_selector()
Expand Down
12 changes: 0 additions & 12 deletions tests/functional/artifacts/expected_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,6 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False):
"group": None,
"schema": my_schema_name,
"database": model_database,
"defer_relation": None,
"alias": "model",
"description": "The test model",
"columns": {
Expand Down Expand Up @@ -372,7 +371,6 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False):
"group": None,
"schema": alternate_schema,
"database": project.database,
"defer_relation": None,
"alias": "second_model",
"description": "The second test model",
"columns": {
Expand Down Expand Up @@ -457,7 +455,6 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False):
"schema": my_schema_name,
"database": project.database,
"alias": "seed",
"defer_relation": None,
"description": "The test seed",
"columns": {
"id": {
Expand Down Expand Up @@ -579,7 +576,6 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False):
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"database": project.database,
"group": None,
"defer_relation": None,
"depends_on": {
"macros": [],
"nodes": ["seed.test.seed"],
Expand Down Expand Up @@ -926,7 +922,6 @@ def expected_references_manifest(project):
"nodes": ["source.test.my_source.my_table"],
},
"deprecation_date": None,
"defer_relation": None,
"description": "",
"docs": {"node_color": None, "show": True},
"fqn": ["test", "ephemeral_copy"],
Expand Down Expand Up @@ -992,7 +987,6 @@ def expected_references_manifest(project):
"nodes": ["model.test.ephemeral_copy"],
},
"deprecation_date": None,
"defer_relation": None,
"description": "A summmary table of the ephemeral copy of the seed data",
"docs": {"node_color": None, "show": True},
"fqn": ["test", "ephemeral_summary"],
Expand Down Expand Up @@ -1061,7 +1055,6 @@ def expected_references_manifest(project):
"nodes": ["model.test.ephemeral_summary"],
},
"deprecation_date": None,
"defer_relation": None,
"description": "A view of the summary of the ephemeral copy of the seed data",
"docs": {"node_color": None, "show": True},
"fqn": ["test", "view_summary"],
Expand Down Expand Up @@ -1145,7 +1138,6 @@ def expected_references_manifest(project):
},
},
"config": get_rendered_seed_config(),
"defer_relation": None,
"depends_on": {"macros": []},
"description": "The test seed",
"docs": {"node_color": None, "show": True},
Expand Down Expand Up @@ -1180,7 +1172,6 @@ def expected_references_manifest(project):
"config": get_rendered_snapshot_config(target_schema=alternate_schema),
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"database": model_database,
"defer_relation": None,
"depends_on": {"macros": [], "nodes": ["seed.test.seed"]},
"description": "",
"docs": {"node_color": None, "show": True},
Expand Down Expand Up @@ -1509,7 +1500,6 @@ def expected_versions_manifest(project):
"constraints": [],
"sources": [],
"depends_on": {"macros": [], "nodes": []},
"defer_relation": None,
"description": "A versioned model",
"deprecation_date": ANY,
"docs": {"node_color": None, "show": True},
Expand Down Expand Up @@ -1580,7 +1570,6 @@ def expected_versions_manifest(project):
"contract": {"checksum": None, "enforced": False, "alias_types": True},
"sources": [],
"depends_on": {"macros": [], "nodes": []},
"defer_relation": None,
"description": "A versioned model",
"deprecation_date": None,
"docs": {"node_color": None, "show": True},
Expand Down Expand Up @@ -1634,7 +1623,6 @@ def expected_versions_manifest(project):
],
},
"deprecation_date": None,
"defer_relation": None,
"description": "",
"docs": {"node_color": None, "show": True},
"fqn": ["test", "ref_versioned_model"],
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/contracts/graph/test_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -1045,10 +1045,10 @@ def test_merge_from_artifact(self):
# new node added should not be in original manifest
assert "model.root.nested2" not in original_manifest.nodes

# old node removed should not have state relation in original manifest
# old node removed should not have defer_relation in original manifest
assert original_manifest.nodes["model.root.nested"].defer_relation is None

# for all other nodes, check that state relation is updated
# for all other nodes, check that defer_relation is updated
for k, v in original_manifest.nodes.items():
if v.defer_relation:
self.assertEqual("other_" + v.database, v.defer_relation.database)
Expand Down

0 comments on commit 7854bfa

Please sign in to comment.