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

Add paginated access to children of nodes #6

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Conversation

InnocentBug
Copy link

@InnocentBug InnocentBug commented Jun 27, 2024

Before this PR it was not possible to access children attributes of nodes.

After this PR it should be possible to access any children nodes with a paginated access.

Notes

This adds a resource to the SDK that allows the API call to find all children of a node.
I only implemented the Sync version of this access.
And oriented myself on the search resource.
It seems to work, but I hope I got the design right.

Tests

Added a new test that extensively tests the capabilities of the child pagination.
Extended some of the existing tests to use children access instead of get.

TODO before PR gets merged

[] return not a list of dicts, but a list of node objects
@brili can you help me of how to convert the dictionaries, I get from search result into nodes?
[x] consider caching the child paginators
At the moment, every time a user accesses a child attribute a new child paginator is created.
That works fine, but isn't efficient with many attribute calls.

We could store the child paginators in the class. Like so:

            # TODO consider a caching of these paginators
            if key in self.children:
                child_paginator = cript.resources.child.ChildPaginator(self, key)
                
                ###
                self.__dict__[key] = child_paginator
                ###
                
                return child_paginator
            else:
                raise AttributeError(key)

This would work, because on repeated access the attribute exists now.

I didn't just want to add them as attributes right now, because I wasn't sure how to avoid possible async between the child attributs in the paginator and the objects in children attribute.

[] Also, I am not sure if this works for children that are present on CRIPT, but not necessarily in python.
Mainly is this line

if key in self.children:

correct?
I expect it to be True if key is a possible child attribute, and to be False else.
But does the node always know all its possible children attributes or only the locally established ones?

TODO not included in this PR

Consider writing access.
What should the behavior be if someone modifies an attribute of a node.
For example, if I add a material to an existing python project like

proj1.material += [new_material_node]

I think the behavior should ultimately be, that the new node gets added to the project, and saved into CRIPT.
And should work for all sub-node types, not just project and material
That does not happen at the moment.
In fact, this may end in undesired behavior right now without error.

@brili
Thank you I appreciate your insights.

@InnocentBug InnocentBug self-assigned this Jun 27, 2024
@InnocentBug
Copy link
Author

@Ardi028 for visibility

@InnocentBug
Copy link
Author

Update:

Using the existing child retrieval resources and remove the duplicate.

Caching the child node paginators as attributes.

Missing #7 to allow full node results instead of dicts.

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