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

Feature: allow to exclude relations on object get #881

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

psrok1
Copy link
Member

@psrok1 psrok1 commented Oct 11, 2023

Your checklist for this pull request

  • I've read the contributing guideline.
  • I've tested my changes by building and running the project, and testing changed functionality (if applicable)
  • I've added automated tests for my change (if applicable, optional)
  • I've updated documentation to reflect my change (if applicable)

What is the current behaviour?

  • Object.access() eagerly loads all parents to include in the original relationship only these items that are accessible for currently logged user
  • This works awfully slow if accessed object has lots of parents

What is the new behaviour?

  • Added exclude_relations flag to GET /<object_type>/<object_dhash> endpoints that excludes relations from query result, loading only basic properties, attributes and tags.
  • Object.access() no longer loads parents so they're lazy-loaded (or eagerly loaded explicitly) and are not filtered by permissions. Internal code doesn't rely on that, but it may affect plugins

TODO
Right now we have breaking change because POST /<object_type>/<object_dhash> doesn't return relations info. Should we add another flag for that?

Test plan

Permission checks should be already covered by current tests

@psrok1 psrok1 force-pushed the feature/exclude-relations-on-object-get branch from 796c25d to 7863c0f Compare October 11, 2023 16:50
@yankovs
Copy link
Contributor

yankovs commented Nov 23, 2023

Hey! :)
Great work and this is a very welcome feature. We actually struggle with this exact issue since we have samples/configs that are 2 years old at this point in MWDB and have many connections and API requests on them result in timeout (this affects places such as karton-mwdb-reporter).

How do you think this change will affect MWDB? Will it make the parents/children view just not render? If it works like that it sounds reasonable to me. From my experience with a pretty big MWDB instance places like parents/children and Karton analysis box are the main hotspots in the view and I think worth considering lazy loading either via an explicit request (like the relations graph) or via pagination (which I know happens in children box, but not sure if it truly keeps an iterator and lazy loads).

Regarding breaking changes in POST, I think either way is fine honestly. I think relying on relationships that return from post request is a pretty niche use case and I believe that if MWDBlib has a way to specify a flag to give relations just like you mentioned it would be sufficient for users of the system.

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.

None yet

2 participants