add handling for irresolvable conflicts #4939
Conversation
github-actions
bot
added
the
group/backend
CodSpeed Performance ReportMerging #4939 will not alter performanceComparing Summary
|
ajtmccarty
force-pushed
the
ajtm-11142024-irresolvable-conflicts
branch
from
39530fc
to
df49784
Compare
| @@ -18,6 +18,7 @@ async def calculate_diff( | |||
| diff_branch: Branch, | |||
| from_time: Timestamp, | |||
| to_time: Timestamp, | |||
| is_first_diff: bool = False, | |||
There was a problem hiding this comment.
the is_first_diff and include_unchanged args are used to keep UNCHANGED values around until a little later in the diff incremental calculation process where they are used for setting previous values
There was a problem hiding this comment.
the changes in here are to exclude diff elements that have been reset to the pre-branch values
There was a problem hiding this comment.
the changes in here are to remove conflicts from nodes that are not in the diff for the base branch b/c if the diff element is not in the base diff, then it has not changed or it has been reset to its pre-diff value
| @@ -62,8 +63,8 @@ async def calculate_diff( | |||
| await base_diff_query.execute(db=self.db) | |||
| for query_result in base_diff_query.get_results(): | |||
| diff_parser.read_result(query_result=query_result) | |||
|
|
|||
| diff_parser.parse() | |||
| include_unchanged = not is_first_diff | |||
There was a problem hiding this comment.
Not a big deal but was thinking if is_first_diff could be worded differently so that we avoid this double negative situation here. Seems like it's mostly used in this location.
| ) | ||
| combined_relationships.add(combined_relationship) | ||
| combined_relationship_elements = {cre for cre in combined_relationship_elements if cre.properties} | ||
| includes_parent = self._child_parent_uuid_map.get(node_id, (None, None))[1] == later_relationship.name |
There was a problem hiding this comment.
This part looks a bit hairy, might it be cleaner to have a method that takes the node_id as input and optionally returns the secondary value of the tuple if the node exists in self._child_parent_uuid_map? "_get_parent_rel_name" or so?
ajtmccarty
force-pushed
the
ajtm-11142024-irresolvable-conflicts
branch
from
d76c6ba
to
be716c3
Compare
PR is too big, sorry
going to save some remaining changes for a follow-up PR
add the ability to mark certain diff conflicts as irresolvable. right now, this is all node-level delete conflicts. for example, if I delete a node on
mainand update it onbranch, we will block merging until the conflict is resolved manually, either by deleting the node onbranchor undoing the changes onbranchthis required some changes to
DiffCombinerandDiffConflictsEnricherand some new tests b/c they were not correctly handling a manual undo of a conflict in some cases