add validation for branch merge #4767
Conversation
github-actions
bot
added
the
group/backend
CodSpeed Performance ReportMerging #4767 will degrade performances by 24.25%Comparing Summary
Benchmarks breakdown
|
ajtmccarty
force-pushed
the
ajtm-10282024-validate-merge
branch
from
78b794d
to
c319d71
Compare
| registry.schema.set_schema_branch(name=obj.name, schema=updated_schema) | ||
| obj.update_schema_hash() | ||
| await obj.save(db=service.database) | ||
| async with lock.registry.global_graph_lock(): |
There was a problem hiding this comment.
just moving this all inside of a global graph lock
| and conflict.selected_branch is ConflictSelection.DIFF_BRANCH | ||
| and conflict.base_branch_value | ||
| and conflict.base_branch_action in (DiffAction.ADDED, DiffAction.UPDATED) | ||
| ): |
There was a problem hiding this comment.
if there is a conflict and the diff version is selected, we need to delete the version on main, which we were not doing before
| def get_all_conflicts(self) -> list[EnrichedDiffConflict]: | ||
| return [prop.conflict for prop in self.properties if prop.conflict] | ||
| def get_all_conflicts(self) -> dict[str, EnrichedDiffConflict]: | ||
| return {prop.path_identifier: prop.conflict for prop in self.properties if prop.conflict} |
There was a problem hiding this comment.
updates to return a map of path identifier to EnrichedDiffConflict
| @@ -390,7 +390,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: | |||
| WHERE type(r_prop) = prop_type | |||
| AND r_prop.status = prop_rel_status | |||
| AND r_prop.from <= $at | |||
| AND (r_prop.to >= $at OR r_prop.to IS NULL) | |||
| AND (r_prop.to > $at OR r_prop.to IS NULL) | |||
There was a problem hiding this comment.
this off-by-one time error was causing a bug where the merge would delete an edge that it had just updated because the to time was set to $at
| @@ -226,50 +229,32 @@ async def validate_graph(self) -> list[DataConflict]: | |||
| async def merge( | |||
| self, | |||
| at: Optional[Union[str, Timestamp]] = None, | |||
| conflict_resolution: Optional[dict[str, bool]] = None, | |||
| conflict_resolution: Optional[dict[str, bool]] = None, # pylint: disable=unused-argument | |||
There was a problem hiding this comment.
I will completely remove conflict_resolution in my next PR or 2
| @@ -96,3 +87,161 @@ async def test_diff_and_merge_schema_with_default_values( | |||
| assert new_bool_attr.default_value is False | |||
| new_str_attr = car_schema_main.get_attribute(name="nickname") | |||
| assert new_str_attr.default_value == "car" | |||
|
|
|||
There was a problem hiding this comment.
a few new tests to make sure that the merge is correctly handling resolved conflicts
fixes #4595
IFC-767
adds validation to the
BranchMergemutation to prevent merging a branch with constraint violations or diff conflicts