-
Notifications
You must be signed in to change notification settings - Fork 181
Fix running sandboxes pagination #1180
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
Fix running sandboxes pagination #1180
Conversation
0902b6a to
41427ad
Compare
41427ad to
deeb029
Compare
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.
wouldn't it be more error prone to sort in the filter based on cursor function?
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.
Bug: Sandbox Pagination Fails Due to Premature Filtering
Pagination is incorrect because cursor filtering is applied to individual sandbox lists before the final merge and sort. Also, paused sandboxes are prematurely limited at the database level, which can exclude them from the final paginated results.
packages/api/internal/handlers/sandboxes_list.go#L162-L180
infra/packages/api/internal/handlers/sandboxes_list.go
Lines 162 to 180 in dfe3d7f
| // Filter based on cursor | |
| runningSandboxList = utils.FilterBasedOnCursor(runningSandboxList, cursorTime, cursorID) | |
| sandboxes = append(sandboxes, runningSandboxList...) | |
| } | |
| if slices.Contains(states, api.Paused) { | |
| pausedSandboxList, err := a.getPausedSandboxes(ctx, team.ID, runningSandboxesIDs, metadataFilter, limit, cursorTime, cursorID) | |
| if err != nil { | |
| zap.L().Error("Error getting paused sandboxes", zap.Error(err)) | |
| a.sendAPIStoreError(c, http.StatusInternalServerError, "Error getting paused sandboxes") | |
| return | |
| } | |
| pausingSandboxList := instanceInfoToPaginatedSandboxes(sandboxesInCache[instance.StatePausing]) | |
| pausingSandboxList = utils.FilterSandboxesOnMetadata(pausingSandboxList, metadataFilter) | |
| pausingSandboxList = utils.FilterBasedOnCursor(pausingSandboxList, cursorTime, cursorID) |
I don't think this comment is correct |
Fix running sandboxes pagination. The running sandboxes were not sorted when limit was applied, causing missing sandboxes in the result set and improper token pagination.