Repository navigation
fix(aws): paginate CE recommendations + RDS engine versions (closes #692) - #693
Conversation
) GetReservationPurchaseRecommendation, GetSavingsPlansPurchaseRecommendation, and DescribeDBMajorEngineVersions each silently discarded every page after the first. For large payer accounts, CE returns recommendations across multiple pages, so callers were seeing a fraction of the real result set. All three call sites now drive a token-driven pagination loop. The existing rate-limiter retry loop stays as the inner (per-page) loop. ctx.Err() is checked at the top of each iteration (feedback_ctx_cancel_terminal). The loop terminates on nil OR empty-string token (PR #690 parity). Exceeding maxRecommendationPages/maxEngineVersionPages = 20 returns a diagnostic error citing the service and issue #692. gocyclo relief: fetchSPAllPages + fetchSPPageWithRetry extracted for SP; queryMajorEngineVersionsWithClient + fetchMajorEngineVersionsForEngine extracted for RDS (all stay under cap 15). Three new test groups: multi-page accumulation (3 pages), empty-token terminal (no extra call), and pagination cap error. Updated TestGetRecommendations_ContextCancellation to assert context.Canceled directly -- the new ctx.Err() guard at the top of the pagination loop fires before the rate-limiter so the error path changed correctly.
cristim
added
triaged
|
@coderabbitai review |
|
Warning Review limit reached
Your plan currently allows 2 reviews/hour. Refill in 18 minutes and 9 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
) (#693) GetReservationPurchaseRecommendation, GetSavingsPlansPurchaseRecommendation, and DescribeDBMajorEngineVersions each silently discarded every page after the first. For large payer accounts, CE returns recommendations across multiple pages, so callers were seeing a fraction of the real result set. All three call sites now drive a token-driven pagination loop. The existing rate-limiter retry loop stays as the inner (per-page) loop. ctx.Err() is checked at the top of each iteration (feedback_ctx_cancel_terminal). The loop terminates on nil OR empty-string token (PR #690 parity). Exceeding maxRecommendationPages/maxEngineVersionPages = 20 returns a diagnostic error citing the service and issue #692. gocyclo relief: fetchSPAllPages + fetchSPPageWithRetry extracted for SP; queryMajorEngineVersionsWithClient + fetchMajorEngineVersionsForEngine extracted for RDS (all stay under cap 15). Three new test groups: multi-page accumulation (3 pages), empty-token terminal (no extra call), and pagination cap error. Updated TestGetRecommendations_ContextCancellation to assert context.Canceled directly -- the new ctx.Err() guard at the top of the pagination loop fires before the rate-limiter so the error path changed correctly.
1 participant
Summary
Three AWS API calls silently read only page 1 of their result sets, causing CUDly to emit partial RI/SP recommendations and incomplete RDS engine-version metadata for customers with large payer accounts (closes #692).
Site 1 --
providers/aws/recommendations/client.go(GetReservationPurchaseRecommendation)result.NextPageTokennever read.fetchRIAllPages) drivesNextPageToken; rate-limiter retry (fetchRIPageWithRetry) is the inner loop per-page.Site 2 --
providers/aws/recommendations/parser_sp.go(GetSavingsPlansPurchaseRecommendation)result.NextPageTokennever read.fetchSPAllPagesdrives pagination per plan type;fetchSPPageWithRetryis the inner loop. Outerfor planTypeloop unchanged.Site 3 --
cmd/multi_service_engine_versions.go(DescribeDBMajorEngineVersions)output.Markernever read.fetchMajorEngineVersionsForEnginepaginates viaMarker;queryMajorEngineVersionsWithClientis extracted for testability;parseDBMajorEngineVersionextracted for gocyclo relief.All three sites share the same contract:
ctx.Err()checked at top of each page iteration (terminal, perfeedback_ctx_cancel_terminal).maxRecommendationPages = 20/maxEngineVersionPages = 20cap with diagnostic error citing issue fix(aws/recommendations): paginate CE recommendation + RDS engine-version describe (single-page truncation) #692.nilOR empty-string token (PR fix(purchases): narrow Describe*Offerings + cap pagination (#688) #690 parity).Test plan
TestGetRecommendations_ContextCancellationupdated: pre-cancelled ctx now returnscontext.Canceleddirectly (ctx.Err() fires before rate-limiter, which is correct).providers/aws/recommendations/...andcmd/....gocyclo -over 10clean on all modified files.Closes #692
Refs #688 #691 PR #690