Add pagintion to backfills index and service show pages #422
Conversation
adrw
commented
Feb 26, 2025
•
adrw
commented
| @QueryParam offset: String? = null, | ||
| @QueryParam lastOffset: String? = null, | ||
| ): Response<ResponseBody> { | ||
| val (backfills, nextOffset) = transacter.transaction { session -> |
There was a problem hiding this comment.
nit - readOnly(). Could also use replicaRead since the replication lag should be acceptable for querying live status
There was a problem hiding this comment.
@mpawliszyn does Backfila have read replicas in staging/production?
There was a problem hiding this comment.
I think for now the simplicity is worth more than the performance.
| .newPager( | ||
| idDescPaginator(), | ||
| initialOffset = offset?.let { Offset(it) }, | ||
| pageSize = 12, |
There was a problem hiding this comment.
nit - put the page size in a companion object const (could also make it user configurable)
There was a problem hiding this comment.
Going to punt on this to match the existing style across other callsites, but generally agree with this approach of moving to companion object
| } | ||
| .list(session) | ||
| .newPager( | ||
| idDescPaginator(), |
There was a problem hiding this comment.
Didn't you want to order by updated_at?
There was a problem hiding this comment.
Turns out the old impl just used the id so I'll stick with that pattern since it matches status quo and is easier.
| ?.contents | ||
| } | ||
|
|
||
| val a = "a" |
| @Inject private lateinit var scope: ActionScope | ||
|
|
||
| @Test | ||
| fun happyPath() { |
There was a problem hiding this comment.
Should we inject the BackfillIndexAction and assert the contents of its response for a couple different offset values?
There was a problem hiding this comment.
Ah I'll go ahead and delete this test class, I was just using it to play with the Paginator and have a debugger.
adrw
force-pushed
the
2025-02-26.pagination
branch
from
8791e72 to
37605c3
Compare
adrw
force-pushed
the
2025-02-26.pagination
branch
from
37605c3 to
e7322f4
Compare
