Repository navigation
fix(cli): Split paginate() limits evenly and return Sentry-compatible cursors - #1642
Conversation
…ersion paginate() used 100-row pages for any --limit over 100, so --limit 201 asked for a full third page to return one row. It now uses the fewest requests and splits the limit evenly across them (201 -> 3 x 67). CLI cursors from another format version, or damaged ones, are ignored with a warning and the walk restarts from the first page instead of failing or reaching the API. Stored -c next history is namespaced by CURSOR_FORMAT, so CLI versions never resume from each other's cursors after an upgrade or downgrade. Co-Authored-By: BYK <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
BYK
left a comment
There was a problem hiding this comment.
Would it be possible to make our new cursor format somehow compatible with the old one? And also this new version being able to "salvage" the old cursors? I'm guessing they are just raw cursor from the server so we should be able to do something with them? You might be able to find more tricks/hacks by looking at getsentry/sentry code
A raw OffsetPaginator cursor (`<per_page>:<page>:<is_prev>`) passed to paginate() was paired with the even-split page size for the limit, so the server read the wrong window and skipped or repeated rows. Recover the page size from the cursor instead; other paginators put 0 or a sort value there and are unaffected.
Review feedback: make cursors work across CLI versions in both directions instead of isolating them. paginate() now returns Sentry cursors. A page-boundary cursor is the server's cursor unchanged. A mid-page cursor is the page's cursor with extra zeros in is_prev recording how many rows were already returned (`84:2:000` skips 2). Sentry parses is_prev with int() (Cursor.from_string), so older CLIs and the API accept these and see the skipped rows again rather than missing any. The page size comes back from page-number cursors themselves (OffsetPaginator encodes it), so it no longer needs to live in the cursor. That makes cursors stored by older CLIs resumable, so the CURSOR_FORMAT namespace for stored -c next history is removed and old and new versions share it again. cli1~ cursors from nightly builds are still read, and remain the fallback for cursors Sentry did not issue. Co-Authored-By: BYK <[email protected]>
| * | ||
| * Sentry parses `is_prev` with `int()` (`Cursor.from_string` in | ||
| * `src/sentry/utils/cursors.py`), so `0`, `00`, and `000` all mean "next". | ||
| * {@link paginate} uses the extra zeros to record how many rows of the page |
There was a problem hiding this comment.
Wow, so this means we can have up to 99 extra 0s there? 🤣 - I think its fine and clever but if you can think of a better way, I'd definitely take it.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f9333f0. Configure here.
The cli1~ fallback for server cursors that cannot carry a skip (opaque strings, is_prev=1) was the one path that still produced cursors older CLIs cannot send to the API, and stored -c next history is shared across versions again. Return the page's server cursor instead; resuming returns that page's rows again rather than missing any. cli1~ cursors from nightly builds are still read.
A skip of up to ~99 rows took up to 100 zeros in is_prev. Python's int() also allows a single `_` between digits (PEP 515), so write the skip in binary with `_0` for 1 and `0` for 0 after a leading `0`: 82 becomes `0_00_000_00`. Sentry still reads it as is_prev=0. encodeSkip() documents the format. Co-Authored-By: BYK <[email protected]>

Follow-up to #1625. This fixes two things before
paginate()'s cursors ship in a release.Overfetching on the last page.
paginate()used 100-row pages for any--limitover 100. So--limit 201made three requests of 100 and threw away 99 rows. A fresh walk now uses the same, minimal number of requests and splits the limit evenly across them: 201 → 3 × 67, 250 → 3 × 84, 101 → 2 × 51. A property test checks the request count (ceil(limit / 100)) and that a walk overfetches by less than one row per request.Cursors that work across CLI versions. #1625 returned CLI-only
cli1~<perPage>~<skip>~<server cursor>cursors. Older CLIs send any cursor straight to the API, so they couldn't use those, and the new CLI couldn't trust the raw cursors older CLIs stored. Now every cursorpaginate()returns is a Sentry cursor:is_prev. After a leading0, a 1 bit is_0and a 0 bit is0, most significant first. So84:2:0_00_000_00means page84:2:0, skip 82 (0b1010010), and the worst case is about 15 characters.is_prevwith Python'sint()(Cursor.from_stringinsrc/sentry/utils/cursors.py).int()allows single underscores between digits (PEP 515), so every such string is0, and every endpoint accepts it as a normal next-page cursor.paginate()refetches the page and skips those rows. An older CLI, or anything else that sends the cursor as is, sees those rows again but never misses any.encodeSkip()documents the format.is_prev=1cursor) is returned as is. Resuming refetches that page.OffsetPaginatorcursors (<per_page>:<page>:<is_prev>) already encode it, sopaginate()reads it back, and pasted or stored raw cursors resume with the right window. Other paginators (BasePaginator,GenericOffsetPaginator,SequencePaginator) treatoffsetas a row offset and take any page size. When resuming mid-page, the first request also fetches the rows to skip, so it uses as few requests as possible.Because both directions work, stored
-c nexthistory is shared across versions again: no namespacing, and nothing is discarded on upgrade or downgrade. Cursors stored by older CLIs are raw server cursors and resume correctly.cli1~cursors saved by nightly builds since #1625 are still read but never written. A damaged one is ignored with a warning, and the walk starts from the first page.Known limit: an older CLI that sends a page-number cursor with a different
--limitstill misreads it. That's the pre-#1625 bug, and this PR can't fix it on the old side.Tests:
OffsetPaginator,GenericOffsetPaginator, and a server that ignoresper_page. They check that every row comes back exactly once across-c nextwith changing limits, that every returned cursor is a Sentry cursor (withis_preva Python int literal equal to 0) that the API resumes from without skipping a row, and that a pasted page-number cursor resumes without skipping.cli1~cursors.int()for skips from 1 to 1000.Local checks:
tscand lint pass. CLI unit suite: 11,300 pass. The only failures are 6 chmod/permission tests (completions, code-scanner, file-changes, telemetry, upgrade) that also fail on cleanmainin this sandbox.via BYK.
--
View Junior Session in Sentry