build and leverage child and parent maps to properly evaluate cross project dependencies #145
Conversation
There was a problem hiding this comment.
@dave-connors-3 I feel like this is a solid approach, but I have one question about return logic on build_parent_and_child_maps, and another low-level question about {}.
| for model in self.project.child_map[resource.unique_id]: | ||
| if model in self.project.resources: | ||
| continue |
There was a problem hiding this comment.
This is such a better solution!
dbt_meshify/dbt_projects.py
Outdated
| if hasattr(self.manifest, "child_map") and hasattr(self, "parent_map"): | ||
| return |
There was a problem hiding this comment.
I'm not sure that I understand what this logic is here for. Can I get a quick explanation?
There was a problem hiding this comment.
this logic looks a little drunk tbh -- let me double check whether this was a type check thing that may be eschewed altogether
There was a problem hiding this comment.
I removed the self.child_map = {} and self.parent_map = {} from the init method, and removed this. IIRC i was battling with mypy when i was writing this manifest method in a slightly different order. I think this should be sufficient!
dbt_meshify/dbt_projects.py
Outdated
| for resource in self.resources: | ||
| if resource.startswith("test"): | ||
| continue | ||
| all_children.update(self.child_map.get(resource, {})) |
There was a problem hiding this comment.
I'm not sure if {} evaluates as an empty set, or an empty dict. Might want to confirm this.
There was a problem hiding this comment.
in fact, the maps return lists! updated the types accordingly, and confirmed that a list is an acceptable arg to the Set.update() method
resolves #143
I will also split an additional issue out to track the question of "what happens if a downstream node has two cross-project parents?
this PR adds:
child_mapandparent_mapobjects into dbt projectsupdate_child_refsto only evaluate children of the model called in the split operation (the original motivation of this PR!)