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

Simplify retrieve #420

Merged
merged 19 commits into from
Oct 2, 2024
Merged

Simplify retrieve #420

merged 19 commits into from
Oct 2, 2024

Conversation

ssssarah
Copy link
Contributor

@ssssarah ssssarah commented Aug 3, 2024

  • 2 (main+metadata through resource endpoint) or 3 (main fail through resource endpoint, main+metadata through resolver endpoint) requests were necessary to retrieve resources:

  • Remove async force complete for retrieval of one resource. Synchronous requests are now used, since the body of the function is simpler, a little bit of duplication doesn't hurt as much.

  • To avoid the client session staying open too long, leading to a ServerDisconnectedError. + all tasks/resources no longer share the same client session (a batch of tasks/resources per client session)

  • When using the resource endpoint, the annotate query parameter only works for regular resources. It fails if we attempt to retrieve a schema with it. The best thing would be to use the schema endpoint with the annotate query parameter but that isn't supported by delta yet. So the mechanism to retrieve data (not source) + metadata, then data (source) without metadata, and then to merge them has been kept in a retrieve_schema method.

  • Allow to return annotated payload for schemas nexus#5079 RDFModel's validate uses the resource retrieve implementation to get schemas and ontologies. It was not possible to use the annotate query parameter in order to retrieve schemas through the resource endpoint until this change in the API.

@ssssarah ssssarah changed the base branch from master to parallelize_validate August 3, 2024 09:24
@ssssarah ssssarah self-assigned this Aug 3, 2024
@ssssarah ssssarah marked this pull request as ready for review August 3, 2024 09:35
@ssssarah ssssarah requested review from MFSY, crisely09 and lecriste August 6, 2024 12:02
@ssssarah ssssarah force-pushed the parallelize_validate branch from 41c7fcf to b68c35b Compare August 16, 2024 09:17
@codecov-commenter
Copy link

codecov-commenter commented Aug 16, 2024

Codecov Report

Attention: Patch coverage is 9.90991% with 100 lines in your changes missing coverage. Please review.

Project coverage is 74.01%. Comparing base (4a0ee65) to head (da98e7d).

Files with missing lines Patch % Lines
kgforge/specializations/stores/bluebrain_nexus.py 6.15% 61 Missing ⚠️
...cializations/stores/nexus/batch_request_handler.py 13.33% 39 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #420      +/-   ##
==========================================
- Coverage   74.19%   74.01%   -0.18%     
==========================================
  Files         106      106              
  Lines        6998     7020      +22     
==========================================
+ Hits         5192     5196       +4     
- Misses       1806     1824      +18     
Flag Coverage Δ
unittests 74.01% <9.90%> (-0.18%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ssssarah ssssarah changed the base branch from parallelize_validate to master August 16, 2024 12:56
@ssssarah ssssarah changed the base branch from master to parallelize_validate August 16, 2024 12:57
@ssssarah ssssarah changed the base branch from parallelize_validate to master August 16, 2024 12:58
@crisely09
Copy link
Contributor

When using the resource endpoint, the annotate query parameter only works for regular resources. It fails if we attempt to retrieve a schema with it. The best thing would be to use the schema endpoint with the annotate query parameter but that isn't supported by delta yet. So the mechanism to retrieve data (not source) + metadata, then data (source) without metadata, and then to merge them has been kept in a retrieve_schema method.

BlueBrain/nexus#5079 RDFModel's validate uses the resource retrieve implementation to get schemas and ontologies. It was not possible to use the annotate query parameter in order to retrieve schemas through the resource endpoint until this change in the API.

Aren't these two points contradicting each other? if now the annotate query parameter is available for schemas, then the retrieve_schema could use the simplified retrieval?

@ssssarah ssssarah merged commit ff61956 into master Oct 2, 2024
2 checks passed
@crisely09 crisely09 deleted the simplify_retrieve branch October 2, 2024 09:17
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.

Annotated source does not behave as expected
3 participants