Add TokenPagination field to ListEgress API and RPC#1556
Add TokenPagination field to ListEgress API and RPC#1556biglittlebigben wants to merge 4 commits into
Conversation
🦋 Changeset detectedLatest commit: d9e5710 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Add TokenPagination field to enhance ListEgress functionality.
| string room_name = 1; // (optional, filter by room name) | ||
| string egress_id = 2 [(logger.name) = "egressID"]; // (optional, filter by egress ID) | ||
| bool active = 3; // (optional, list active egress only) | ||
| TokenPagination page_token = 4; |
There was a problem hiding this comment.
we probably want Pagination unless we don't want to allow users to set limit lower than the default
There was a problem hiding this comment.
Ingress is using TokenPagination for the same purpose. I believe we've been leaning towards TokenPagination in most APIs recently to keep the token anonymous, which allows us to change its semantics more easily. The exception would be if there is a reason to allow the caller to define offsets or page sizes other than the default. I don't believe there is a strong incentive to do this here.
There was a problem hiding this comment.
if we set the page size too big we're overscanning for customers who don't care and if we set it too small they're just going to hammer the api until they get what they want and our api quotas are so generous as to be basically useless for preventing abuse...
There was a problem hiding this comment.
That would be an argument to add a limit parameter to the request. We can add it at the top level while keeping token pagination which has the pro of making the token anonymous, and easier to change if needed. The 2 cursors particularly are confusing and users may think they need to provide them somehow.
That being said, I am not convinced we should add a limit field proactively. From the customer's perspective, there is little incentive to request pages smaller than the maximum we are willing to provide so we should probably just return whatever makes sense to us (currently 300 items).
3 participants
No description provided.