Skip to content
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

Fix problems with the node cache #441

Merged
merged 5 commits into from
Mar 12, 2024
Merged

Fix problems with the node cache #441

merged 5 commits into from
Mar 12, 2024

Conversation

InnocentBug
Copy link
Collaborator

Description

Now the node UUID cache actually works as intended.
Users can only have one python object with the same UUID.

Changes

Added a functionality that allows you to circumvent this cache explictly.
This should be a feature that users don't really use.

Known Issues

Notes

Checklist

  • My name is on the list of contributors (CONTRIBUTORS.md) in the pull request source branch.
  • I have updated the documentation to reflect my changes.
  • My code changes have been verified by automated tests and pass all relevant test scenarios.

@InnocentBug InnocentBug self-assigned this Mar 12, 2024
Copy link

trunk-io bot commented Mar 12, 2024

🚫 This pull request was closed and has been removed from the merge queue (details).

@InnocentBug
Copy link
Collaborator Author

@duboyal
This fixes a bug, and allows you to load the old tree of a project for comparison as discussed.
What you do is the following.

old_project_paginator = cript_api.search(node_type=cript.Project, search_mode=cript.SearchModes.UUID, value_to_search=str(new_project.uuid))
old_project_paginator.auto_load_nodes = False

old_json = next(old_project_paginator)
old_project_node, old_tree_cache = cript.load_nodes_from_json(normal_serial, _use_uuid_cache=dict())

And then you can go about it as discussed.

Key here is, that you override the normal UUID cache with an empty dictionary in the load_nodes_from_json call.
And second, the cache that is returned from the call, is the UUID cache that is assembled during the load_call.

This should automatically correspond to what we called the old_tree_map that we discussed.

@@ -349,7 +350,22 @@ def load_nodes_from_json(nodes_json: Union[str, Dict]):
# but catches a lot of odd cases. And at the moment performance is not bottle necked here
if not isinstance(nodes_json, str):
nodes_json = json.dumps(nodes_json)
return json.loads(nodes_json, object_hook=node_json_hook)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💟

@@ -32,16 +29,26 @@ class JsonAttributes(BaseNode.JsonAttributes):

_json_attrs: JsonAttributes = JsonAttributes()

def __new__(cls, *args, **kwargs):
uuid: Optional[str] = kwargs.get("uuid")
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fixes the bug that we had before.

Fun playing with something new like the __new__

duboyal
duboyal previously approved these changes Mar 12, 2024
@InnocentBug
Copy link
Collaborator Author

/trunk merge

@InnocentBug InnocentBug merged commit 6b1738e into develop Mar 12, 2024
13 checks passed
@InnocentBug InnocentBug deleted the load-old-tree branch March 12, 2024 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants