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 key range parameters to REST API v2 #5656

Closed
wants to merge 1 commit into from

Conversation

dimas-b
Copy link
Member

@dimas-b dimas-b commented Dec 8, 2022

  • In "diff" API

  • In "get entries"

  • New "get contents" endpoint for range queries

This is just an API-level change to reserve REST paths and query
parameters. Server-side implementation to be done separately.

Closes #5591

@dimas-b dimas-b added the pr-native run native test label Dec 8, 2022
@dimas-b dimas-b requested a review from snazy December 8, 2022 00:57
@codecov
Copy link

codecov bot commented Dec 8, 2022

Codecov Report

Base: 83.91% // Head: 79.26% // Decreases project coverage by -4.64% ⚠️

Coverage data is based on head (4b17846) compared to base (09962ca).
Patch coverage: 32.65% of modified lines in pull request are covered.

❗ Current head 4b17846 differs from pull request most recent head 60f7dc3. Consider uploading reports for the commit 60f7dc3 to get more accurate results

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5656      +/-   ##
============================================
- Coverage     83.91%   79.26%   -4.65%     
- Complexity        0     3724    +3724     
============================================
  Files            30      536     +506     
  Lines          1473    16515   +15042     
  Branches        238     1613    +1375     
============================================
+ Hits           1236    13090   +11854     
- Misses          171     2804    +2633     
- Partials         66      621     +555     
Flag Coverage Δ
java 78.71% <32.65%> (?)
javascript 82.91% <ø> (?)
python 83.91% <ø> (ø)

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

Impacted Files Coverage Δ
...a/org/projectnessie/client/api/GetDiffBuilder.java 0.00% <ø> (ø)
...g/projectnessie/client/http/v2api/HttpGetDiff.java 0.00% <0.00%> (ø)
...ava/org/projectnessie/api/v2/http/HttpTreeApi.java 0.00% <0.00%> (ø)
...va/org/projectnessie/api/v2/params/DiffParams.java 0.00% <0.00%> (ø)
...ctnessie/error/NessieContentNotFoundException.java 42.85% <0.00%> (ø)
...ain/java/org/projectnessie/model/DiffResponse.java 0.00% <ø> (ø)
...g/projectnessie/client/http/v1api/HttpGetDiff.java 29.41% <23.07%> (ø)
...ojectnessie/client/builder/BaseGetDiffBuilder.java 15.78% <27.27%> (ø)
...rg/projectnessie/api/v2/params/AbstractParams.java 58.33% <100.00%> (ø)
...org/projectnessie/api/v2/params/EntriesParams.java 87.50% <100.00%> (ø)
... and 506 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

* In "diff" API

* In "get entries"

* New "get contents" endpoint for range queries

This is just an API-level change to reserve REST paths and query
parameters. Server-side implementation to be done separately.

Closes projectnessie#5591
@dimas-b dimas-b marked this pull request as ready for review December 8, 2022 19:37
@dimas-b
Copy link
Member Author

dimas-b commented Dec 8, 2022

@snazy : do you want range params for reference endpoints too?

@snazy
Copy link
Member

snazy commented Dec 14, 2022

@snazy : do you want range params for reference endpoints too?

Yes, that would be good. It would also be nicer to the new storage stuff to change the maxKey parameter from exclusive to _inclusive. We can technically encode a prefix-query with the min and max parameters:

  • from...to : minKey defines the start (inclusive) and maxKey the end (inclusive)
  • from : minKey defines the start (inclusive)
  • to : maxKey defines the end (inclusive)
  • prefix : minKey == maxKey define the prefix for the returned keys. "min equal to max" doesn't really make sense for a query and should be performed via a point-query (get-content) - so we could abuse this case. From a usability perspective if would probably make sense to have a separate parameter for this though.

@snazy
Copy link
Member

snazy commented Dec 14, 2022

@snazy : do you want range params for reference endpoints too?

It could make sense to have this kind of query for references, so yes - especially the prefix one.

@snazy
Copy link
Member

snazy commented Jan 25, 2023

Shall we continue on this one?
We can omit New "get contents" endpoint for range queries for now, but have range-queries for getAllReferences

@snazy
Copy link
Member

snazy commented May 9, 2023

Closing with its successor #6743

@snazy snazy closed this May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-native run native test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add start/end key parameters to v2 API methods
2 participants