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

monitor containers for modified entities #51

Merged
merged 27 commits into from
Feb 15, 2022

Conversation

hhunterzinck
Copy link
Collaborator

@hhunterzinck hhunterzinck commented Feb 6, 2022

Fixes #23

  • added _traverse: traversal function that returns any descendants of a given entity type (retained to enhance later functionality)
  • implemented _find_modified_entities_container: checks container and descendants for modification
  • removed test for unimplemented functions
  • added tests in TestModifiedContainer class

@thomasyu888
Copy link
Member

thomasyu888 commented Feb 6, 2022

Why not use walk? It basically does the same thing as what you've written in _traverse.

I think there is missing functionality like include_types for synapseutils.walk

@hhunterzinck
Copy link
Collaborator Author

re: Why not use synapseutils.walk? The synapseutils.walk returns a dictionary in which one of the values is a nested list of descendant entities that requires parsing to extract just the descendant entity Synapse IDs as a simple list. The extra parsing step in addition to the lack of the include_types functionality made implementing the _traverse function here explicitly more straight-forward and flexible.

synid_children = syn.getChildren(parent=synid_root, includeTypes=include_types_mod)
for synid_child in synid_children:
synid_desc.extend(
_traverse(
Copy link
Member

@thomasyu888 thomasyu888 Feb 9, 2022

Choose a reason for hiding this comment

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

I just remembered, in Python there is a max recursion limit - limit of calling a function recursively ~1000 times. So you may get into trouble here because if you do a getChildren on a folder with 1000 files, you're actually calling _traverse 1000 times. We may want to do something similar as synapseutils.walk where we only call _traverse when it is a container.

Then again - for those cases, we should encourage the use of file views...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a check for the entity type of the children to limit recursive calls. Now, if a child entity is neither a folder nor a project, the child entity type is added directly to the descendant list after checking that it is a requested entity type.


synid_children = syn.getChildren(parent=synid_root, includeTypes=include_types_mod)
for synid_child in synid_children:
entity = syn.get(synid_child["id"], downloadFile=False)
Copy link
Member

@thomasyu888 thomasyu888 Feb 10, 2022

Choose a reason for hiding this comment

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

I'm not sure this syn.get isn't necessary. Print out synid_child. I think concreteType is part of it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Almost. Child entity reference 'concreteType' as 'type'. But I've removed the redundant syn.get

@hhunterzinck hhunterzinck merged commit 1e92b1b into develop Feb 15, 2022
@hhunterzinck hhunterzinck deleted the gh-23-monitor-container branch February 15, 2022 16:00
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.

Add support to monitor containers (Projects and Folders)
2 participants