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

Libpitt/606 grabbing activity props #627

Merged
merged 6 commits into from
Feb 13, 2025

Conversation

libpitt
Copy link
Contributor

@libpitt libpitt commented Feb 13, 2025

Change log:

  1. Set up a global property _schema_properties which is a dict with entity property names as keys so as to facilitate constant time access. Since we have various entities, it would be inefficient to loop every entity, then loop every property and match in order to group.
  2. Simplify logic in trigger get_dataset_organ_and_source_info and speed up response

@libpitt libpitt marked this pull request as ready for review February 13, 2025 15:09
@libpitt libpitt requested a review from maxsibilla as a code owner February 13, 2025 15:09
@libpitt
Copy link
Contributor Author

libpitt commented Feb 13, 2025

So yesterday when I was testing the POST /descendants endpoint, in the results were both a Publication and a Dataset. In the filter I had requested title. But since title is a property of both Publication and Dataset, the previous code had a key collision. In Dataset, it’s a trigger, in Publication a neo4j property. In order to avoid loss, would have to catch the settings of each property per Entity type. This is right away going to be O n^x time unless we use a dictionary where we can get details about a property via a key, which is constant time. But we don’t want to be creating that dictionary every time we do a POST filter_properties type call. Hence the global variable _schema_properties and that method group_schema_properties_by_name that runs on startup.

@maxsibilla maxsibilla merged commit dfcf41c into dev-integrate Feb 13, 2025
2 checks passed
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