fix: paginate ListNotCompletedDeployments in pipedv1#6727
fix: paginate ListNotCompletedDeployments in pipedv1#6727sridhar-panigrahi wants to merge 3 commits intopipe-cd:masterfrom
Conversation
|
@khanhtc1202 updated the patch at #6727 — went with the proto approach as originally proposed. Added cursor and page_size to the request, forwarded them server-side, and the pipedv1 store now loops until it gets an empty cursor. v0 store untouched. Let me know if anything needs changing! |
piped was calling ListNotCompletedDeployments once per sync tick and throwing away the cursor on the response. If the datastore returned a partial page, every deployment past the first page would be silently skipped until the next tick — showing up as stuck PENDING or PLANNED. Fix: - Add cursor and page_size to ListNotCompletedDeploymentsRequest proto - Forward them in PipedAPI.ListNotCompletedDeployments into the datastore ListOptions so the server honours the client's position - Replace the single RPC call in the pipedv1 deployment store with a loop that follows the cursor until the server returns an empty one, accumulating all deployments before classifying them - Add table-driven tests for the v1 store covering single-page, multi-page, and all deployment status classifications v0 store is intentionally untouched (frozen). Fixes pipe-cd#6696 Signed-off-by: Shridhar Panigrahi <sridharpanigrahi2006@gmail.com>
sridhar-panigrahi
force-pushed
the
fix/pipedv1-list-not-completed-deployments-pagination
branch
from
0dc66c0 to
fb7eff4
Compare
sridhar-panigrahi
changed the title
|
@sridhar-panigrahi please follow the PR template used for pipecd. It is hard to tell why this PR/Change is needed. Look at previous PRs for any idea. |
|
@mohammedfirdouss thanks for the nudge — you're right, the original description didn't make the motivation clear. I've rewritten it to follow the PR template (using #6724 as the reference). Let me know if it reads better now. |
@sridhar-panigrahi This is good, but try and remove the extra details beneath the template "A bit more detail on the changes" |
2 participants
What this PR does:
Makes
pipedactually paginate when it callsListNotCompletedDeployments. The pipedv1 deployment store now loops through all pages using the cursor the server returns, instead of taking only the first page and throwing the cursor away.Why we need it:
Right now
pipedcallsListNotCompletedDeploymentsonce per sync tick and ignores the cursor on the response. If the datastore returns a partial page, anything past that boundary just gets dropped on the floor — those deployments look stuck inPENDINGorPLANNEDuntil the overall volume drops enough to fit in a single page. I hit this while reading through the store code and traced it back to #6696.Which issue(s) this PR fixes:
Fixes #6696
Does this PR introduce a user-facing change?:
A bit more detail on the changes
service.proto— addedpage_size(int32) andcursor(string) toListNotCompletedDeploymentsRequest. Regenerated stubs withmake gen/code.pkg/app/server/grpcapi/piped_api.go— forwardsreq.PageSize→opts.Limitandreq.Cursor→opts.Cursorso the server respects the client's position.pkg/app/pipedv1/apistore/deploymentstore/store.go— swaps the single RPC for a loop that feeds each response's cursor back into the next request, until the server returns an empty one. All pages get accumulated before classifying into pending/planned/running.pkg/app/pipedv1/apistore/deploymentstore/store_test.go(new) — table-driven tests covering empty, single-page, and multi-page responses, plus the status classification paths, using a fake API client.v0 store is left alone since it's frozen.
Signed-off-by: Shridhar Panigrahi sridharpanigrahi2006@gmail.com