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

REST V2: Key ranges for entries + diff #6743

Merged
merged 3 commits into from May 17, 2023
Merged

Conversation

snazy
Copy link
Member

@snazy snazy commented May 4, 2023

Builds upon the work in #5656.

However, the "get range of contents" has been removed, because it's tricky to add it to the current Nessie Java API and since it's probably not a very useful functionality. It would also require adding paging and min/max-keys to the GetMultipleContentRequest/Response types to leverage the existing paging infrastructure and/or add ContentsParams parameter object - neither works well with the existing code.

Adds the necessary VersionStore changes, which are pretty straight forward.

Fixes #3957
Fixes #5591

@snazy snazy marked this pull request as ready for review May 4, 2023 13:34
@snazy snazy requested a review from dimas-b May 4, 2023 13:34
@snazy snazy force-pushed the key-ranges branch 4 times, most recently from cbdcedb to 20ec663 Compare May 9, 2023 07:01
@snazy snazy force-pushed the key-ranges branch 4 times, most recently from 03c5fcb to 6187611 Compare May 10, 2023 12:16
@snazy
Copy link
Member Author

snazy commented May 11, 2023

Reverted the byte[] change in StoreKey and updated StoreKey.compareTo to respect the 0/1 separated elements to not conflict with (unicode) chars.

@snazy snazy added this to the 0.59.0 milestone May 12, 2023
Copy link
Member

@dimas-b dimas-b left a comment

Choose a reason for hiding this comment

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

Mostly optimization concerns

@snazy snazy force-pushed the key-ranges branch 2 times, most recently from 608020b to 99db9e0 Compare May 13, 2023 10:48
Copy link
Member

@dimas-b dimas-b left a comment

Choose a reason for hiding this comment

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

LGTM overall, but prefix handling seems to contradict (older) docs

dimas-b
dimas-b previously approved these changes May 17, 2023
dimas-b and others added 2 commits May 17, 2023 17:17
* In "diff" API

* In "get entries"

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 previously approved these changes May 17, 2023
@snazy snazy merged commit 9558d8e into projectnessie:main May 17, 2023
23 checks passed
@snazy snazy deleted the key-ranges branch May 17, 2023 16:46
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 start/end key parameters to v2 API methods ContentKey based filter in the Nessie Diff API
2 participants