-
Notifications
You must be signed in to change notification settings - Fork 13
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
Quay API v2 #24
base: main
Are you sure you want to change the base?
Quay API v2 #24
Conversation
creation-date: 2023-07-24 | ||
last-updated: yyyy-mm-dd | ||
status: implementable | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The header should start and end with ---
.
https://github.blog/2013-09-27-viewing-yaml-metadata-in-your-documents/
https://jekyllrb.com/docs/front-matter/
enhancements/api-v2.md
Outdated
cusor-based pagination seems to work best for our current needs. | ||
|
||
Cursor will contain a unique sequential db column to base cursors on along with additional data when required. | ||
(Eg: sorting results on non-unique keys, like datetime) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add examples of the cursor data.
I suspect there will be two options:
{"id": 123}
where 123 is the id of the last item on the return page;{"id": "2023-08-02T12:34:56Z", "n": 32}
where n is how many items were sent to the client with the same id (on the current page and possible on previous pages too).
But I do not want to speculate on this and I want us all to be on the same page. This detail is really important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surely, good point, will add examples.
- value: is what the query will filter for | ||
- multiple supported filters can be added to the query parameters using `&` | ||
|
||
Eg: (GET `/tags?tag_name[like]=quay`, GET `/logs?created_at[lte]=2023-07-20 19:56`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have examples when multiple filters are needed for the UI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a use case of multiple filters in the designs. I want the design to accommodate this as this will be a one time investment. Can be good to have for future needs.
|
||
Sort order will be represented by the query parameter `sort`, ascending order by `+` and descending order by `-`. | ||
Multiple sorts can be passed in the url by adding comma separated keys. | ||
Eg: (GET `/tags?sort=+created_at`, GET `/users?sort=+creation_date,-last_accessed`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will users be able to sort by any column? How do we deal with columns that are not indexed and sorting on which can be terribly slow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thoughts are to restrict sorts on only the cols the UI needs today. If a user makes a request with unsupported col sort, 400 will be sent back.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will document the above.
|
||
Cursor data is extensible and can look like: | ||
``` | ||
{"id": 123} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can a cursor from the request with one set of query parameters be reused with the request with another set of parameters (with different sorting or different filters)?
1. limit: number of items to be returned for the page, max limit set to 100 items per page (based from current Quay API guidelines). | ||
2. prev_cursor/next_cursor | ||
|
||
a. prev_cursor: encrypted string to fetch previous page results |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How will this cursor be generated?
|
||
## Summary | ||
|
||
Currently, only a few APIs have the option to paginate results on the backend. The goal of the proposal is to have a re-usable system to perform actions on the backend and send the curated result on the frontend. Ideally frontend should only display the results and allow the backend to perform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "resources" instead of "APIs" more accurate?
It is difficult to say that one pagination strategy suits all needs. After understanding the pros and cons of various pagination strategies, | ||
cusor-based pagination seems to work best for our current needs. | ||
|
||
Cursor will contain a unique sequential db column to base cursors on along with additional data when required. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this different from what we currently support in terms of pagination?
On some resources, we already have cursor based pagination, where we return an opaque token to be sent on subsequent requests.
How is this proposal different or supposed to improved what's already there, performance or otherwise?
|
||
The following will be expected request parameters for an API that is to be paginated: | ||
1. limit: number of items to be returned for the page, max limit set to 100 items per page (based from current Quay API guidelines). | ||
2. prev_cursor/next_cursor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the use case for a pointer to the previous page?
## Motivation | ||
|
||
In the quay ui, filtering, sorting and occasionally paginating the results happen on the frontend. | ||
This leads to high rendering time on the frontend and thereby a poor user experience. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the implication of delegating that work to the server-side?
Can you add some implementation details w.r.t the database (query changes, indices, ...) involved,
Implement a generic system that is consistent across different endpoints to support the following operations on Quay endpoints: | ||
- pagination | ||
- filtering | ||
- sorting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment above. I assume doing filtering and sorting on the server side vs client-side has some significant performance trade-offs.
|
||
- Proposed path of the endpoints: In an attempt to not change the existing endpoints, new APIs will be under `/api/v2_draft/`. | ||
`v2_draft` will be replaced with `v2` when ready to release. | ||
- Existing decorators like the scope of API, page parameters will be reused where ever required. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this not going to be backward-compatible? If just adding pagination to the current existing resources.
No description provided.