-
Notifications
You must be signed in to change notification settings - Fork 37
node-level create and read user timestamps #7629
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: ajtm-immutable-metadata
Are you sure you want to change the base?
node-level create and read user timestamps #7629
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (4)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
CodSpeed Performance ReportMerging #7629 will not alter performanceComparing Summary
Footnotes |
4ed6377 to
b4ffb64
Compare
b4ffb64 to
ba0f573
Compare
|
|
||
| EVENT_NAMESPACE = "infrahub" | ||
|
|
||
| SYSTEM_USER_ID = "__system__" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default "user ID" to use for updated/created_by/at metadata updates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new, very simple base class to use on Node, Attribute, and Relationship classes to track the new metadata
|
|
||
| raise InitializationError("The node has not been saved yet and doesn't have an id") | ||
|
|
||
| def get_updated_at(self) -> Timestamp | None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replaced by version in MetadataBase
| if (fields and name in fields) or not fields: | ||
| attr: BaseAttribute = getattr(self, name) | ||
| updated_attribute = await attr.save(at=update_at, db=db) | ||
| attr = self.get_attribute(name=name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a more type-hint friendly way of getting an Attribute or RelationshipManager for a node
| result = query.get_result() | ||
|
|
||
| if result and result.get("rb.branch") == branch.name: | ||
| await update_relationships_to([result.get("rb_id")], to=delete_at, db=db) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to remove this wherever I find them
the NodeDeleteQuery should handle this at the database level inside of its cypher query instead of requiring this extra processing in memory and a separate trip to the database
| if self.branch.is_default or self.branch.is_global: | ||
| self.params["node_prop"].update( | ||
| { | ||
| "created_at": at.to_string(), | ||
| "created_by": self.user_id, | ||
| "updated_at": at.to_string(), | ||
| "updated_by": self.user_id, | ||
| } | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we only add these properties to the Node vertex if we are on the default or global branch to make querying for metadata on those branches faster than on user-created branches
| attr_edge_prop_str = "{ branch: attr.branch, branch_level: attr.branch_level, status: attr.status, from: $at, from_user_id: $user_id }" | ||
| attr_vertex_prop_str = "{ uuid: attr.uuid, name: attr.name, branch_support: attr.branch_support" | ||
| if self.branch.is_default or self.branch.is_global: | ||
| attr_vertex_prop_str += ", created_at: $at, created_by: $user_id, updated_at: $at, updated_by: $user_id" | ||
| attr_vertex_prop_str += " }" | ||
|
|
||
| rel_edge_prop_str = "{ branch: rel.branch, branch_level: rel.branch_level, status: rel.status, from: $at, from_user_id: $user_id }" | ||
| rel_edge_prop_str_hierarchy = ( | ||
| "{ branch: rel.branch, branch_level: rel.branch_level, " | ||
| "status: rel.status, hierarchy: rel.hierarchical, from: $at, from_user_id: $user_id }" | ||
| ) | ||
| rel_vertex_prop_str = "{ uuid: rel.uuid, name: rel.name, branch_support: rel.branch_support" | ||
| if self.branch.is_default or self.branch.is_global: | ||
| rel_vertex_prop_str += ", created_at: $at, created_by: $user_id, updated_at: $at, updated_by: $user_id" | ||
| rel_vertex_prop_str += " }" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mostly refactoring to move a bunch of repeated property create strings into one place
and add the updated/created_by/at metadata to them
| WHERE r.branch = $branch | ||
| SET r.to = $at | ||
| SET r.to_user_id = $user_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now handles setting the to time on edges that used to be handled in the update_relationships_to function
|
|
||
| def _add_updated_metadata_to_query(self, branch_filter_str: str) -> None: | ||
| if self.branch.is_default or self.branch.is_global: | ||
| last_update_query = """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can use this very simple addition to the query on the default or global branch
| WITH *, n.updated_at AS updated_at, n.updated_by AS updated_by | ||
| """ | ||
| else: | ||
| last_update_query = """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we have to use this more complex query on a user-created branch b/c we have to look at every edge under the node to find its latest update_at/by
part of IFC-1942
adds created/updated_at/by metadata to nodes when created, read, and deleted
mostly updates logic in the cypher queries to create, delete, and read a node from the database to support the new metadata on edges (
from_user_idandto_user_id) andNodes (updated_at,updated_by,created_at,created_by)logic for writing the data correctly during a non-create update will come later after Attributes and Relationships have the new metadata fully incorporated