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

Return "raw" API parameters in pagination #3412

Merged
merged 3 commits into from
May 3, 2023

Conversation

dbutenhof
Copy link
Member

PBENCH-1133

The GET /datasets response is optimized for sequential pagination, providing a convenient "next_url" string that can be used directly. However if a client wants to support "random access" pagination, this requires that the client parses the URL string in order to modify the offset parameter.

This attempts to make that a bit easier by supplementing the current response payload with a parameters field containing the query parameters JSON object, making it easy to update the offset parameter.

(Making the unit tests work against the normalized parameter list proved a bit challenging and I ended up saving the original "raw" client parameters in the API context so they can be used directly.)

@dbutenhof dbutenhof added Server Dashboard Of and relating to the Dashboard GUI API Of and relating to application programming interfaces to services and functions labels May 2, 2023
@dbutenhof dbutenhof self-assigned this May 2, 2023
webbnh
webbnh previously approved these changes May 2, 2023
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

Looks good, but I really think the raw parameters should be returning the current offset, not the next one...but, I'm willing to 👃 🤏 if you're unconvinced.

Beyond that, I have a nit and some confusion.

npalaska
npalaska previously approved these changes May 2, 2023
PBENCH-1133

The `GET /datasets` response is optimized for sequential pagination, providing
a convenient "next_url" string that can be used directly. However if a client
wants to support "random access" pagination, this requires that the client
parses the URL string in order to modify the `offset` parameter.

This attempts to make that a bit easier by supplementing the current response
payload with a `parameters` field containing the query parameters JSON object,
making it easy to update the `offset` parameter.

(Making the unit tests work against the normalized parameter list proved a bit
challenging and I ended up saving the original "raw" client parameters in the
API `context` so they can be used directly.)
Copy link
Member

@webbnh webbnh left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@siddardh-ra siddardh-ra left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@dbutenhof dbutenhof merged commit 0a5a032 into distributed-system-analysis:main May 3, 2023
@dbutenhof dbutenhof deleted the page branch May 3, 2023 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Of and relating to application programming interfaces to services and functions Dashboard Of and relating to the Dashboard GUI Server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants