Children of multiple boundary nodes #148
Conversation
There was a problem hiding this comment.
@dave-connors-3 This approach seems both necessary and sufficient. I left a suggestion to extract some shared functionality to keep the code DRY. Otherwise, looks good. Great sleuthing on this!
dbt_meshify/utilities/references.py
Outdated
| if current_change_set: | ||
| previous_changes: List[FileChange] = [ | ||
| change | ||
| for change in current_change_set.changes | ||
| if ( | ||
| isinstance(change, FileChange) | ||
| and change.identifier == model_node.name | ||
| and change.operation == Operation.Update | ||
| and change.entity_type == EntityType.Code | ||
| and change.path | ||
| == self.project.parent_project.resolve_file_path(model_node) | ||
| ) | ||
| ] | ||
| previous_change = previous_changes[-1] if previous_changes else None | ||
|
|
||
| change = self.generate_reference_update( | ||
| project_name=self.project.name, | ||
| upstream_node=resource, | ||
| downstream_node=model_node, | ||
| code=model_node.raw_code, | ||
| code=previous_change.data | ||
| if (previous_change and previous_change.data) | ||
| else model_node.raw_code, |
There was a problem hiding this comment.
This approach seems reasonable. Since this filtering is being done here and in linker.py, I think it might be appropriate to extract this logic into a function in this file (references.py) that both this method and the linker.py method can leverage.
There was a problem hiding this comment.
totally agree - would this be a method on the ReferenceUpdater Class, or just a function outside the scope of the class?
There was a problem hiding this comment.
I'm thinking outside the ReferenceUpdater class scope.
There was a problem hiding this comment.
@dave-connors-3 This is an excellent fix. Thanks for the updates. ✅
| if current_change_set: | ||
| previous_change = get_latest_file_change( | ||
| changeset=current_change_set, | ||
| identifier=downstream_manifest_entry.name, | ||
| path=downstream_project.resolve_file_path(downstream_manifest_entry), | ||
| ) | ||
|
|
||
| code_to_update = ( | ||
| previous_change.data | ||
| if (previous_change and previous_change.data) | ||
| else downstream_manifest_entry.raw_code | ||
| ) | ||
|
|
There was a problem hiding this comment.
I really like how this flows! Thanks for making the change.
Resolves #147
This PR adds some state awareness to the split and connect commands so that updates to ref functions are additive and not destructive!
When splitting:
When connecting
I am not completely in love with the approach, so would relish some feedback