Add "other" relation to reffable node classes #7645
Conversation
stu-k
requested review from
ChenyuLInx,
emmyoop and
davidbloss
and removed request for
a team
core/dbt/contracts/graph/nodes.py
Outdated
| @@ -615,6 +619,7 @@ class ModelNode(CompiledNode): | |||
| constraints: List[ModelLevelConstraint] = field(default_factory=list) | |||
| version: Optional[NodeVersion] = None | |||
| latest_version: Optional[NodeVersion] = None | |||
| state_relation: Optional[RelationalNode] = None | |||
There was a problem hiding this comment.
As mentioned in the PR description, state_relation is a very general name that doesn't explain much of what this field holds. Please feel free to suggest better names for this attribute.
There was a problem hiding this comment.
Maybe other_relation, target_relation, cross_environment_relation
There was a problem hiding this comment.
Spoke with Gerda about this name and she thought it made sense for the context it was in.
core/dbt/contracts/graph/nodes.py
Outdated
| @@ -271,16 +271,20 @@ def add_public_node(self, value: str): | |||
|
|
|||
|
|
|||
| @dataclass | |||
| class ParsedNodeMandatory(GraphNode, HasRelationMetadata, Replaceable): | |||
| class RelationalNode(HasRelationMetadata): | |||
There was a problem hiding this comment.
I think it's confusing to use Node in the name here. It's not a Node. These classes are already complicated enough, I don't think it's necessarily a good idea to reuse the existing classes in nodes.py. You're actually reimplementing a Relation class. Do you actually need the separate database/schema/identifier? ("identifier" is what it's actually called in the Relation classes, and makes more sent to me, at least.). I'm not exactly sure how this is going to be used. Would a relation_name (clone_relation_name?) field work?
resolves #7550
Description
As part of
dbt clone#7256, we need to add an additional field to reffable nodes (i.e.ModelNode,SeedNode,SnapshotNode). This field holds a relation to another node that will be used later in thedbt cloneproject to be exposed in a jinja context to enable theclonematerialization. The namestate_relationis very general and we do not need to use this name.This code is taken from an existing draft PR that Jeremy has up.
Checklist
changie newto create a changelog entry