fix: CCAPI provider does not do pagination #678
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mrgrain
requested changes
Jul 2, 2025
Comment on lines
112
to
113
| // eslint-disable-next-line no-console | ||
| console.log(result); |
Contributor
There was a problem hiding this comment.
Suggested change
| // eslint-disable-next-line no-console | |
| console.log(result); |
rix0rrr
force-pushed
the
huijbers/cc-provider-pagination
branch
from
d459e5d to
b84bdfb
Compare
mrgrain
reviewed
Jul 2, 2025
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #678 +/- ##
==========================================
+ Coverage 78.94% 78.96% +0.01%
==========================================
Files 46 46
Lines 7102 7102
Branches 793 797 +4
==========================================
+ Hits 5607 5608 +1
+ Misses 1476 1473 -3
- Partials 19 21 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mrgrain
reviewed
Jul 2, 2025
|
|
||
| // This allows us to error out early, before we have consumed all pages. | ||
| if ((expectedMatchCount === 'at-most-one' || expectedMatchCount === 'exactly-one') && found.length > 1) { | ||
| throw new ContextProviderError(`Found ${found.length} resources matching ${JSON.stringify(propertyMatch)}; expected ${expectedMatchCountText(expectedMatchCount)}. Please narrow the search criteria`); |
Contributor
There was a problem hiding this comment.
The error messages is technically wrong now. Maybe something like?
Suggested change
| throw new ContextProviderError(`Found ${found.length} resources matching ${JSON.stringify(propertyMatch)}; expected ${expectedMatchCountText(expectedMatchCount)}. Please narrow the search criteria`); | |
| throw new ContextProviderError(`Found more than one resources matching ${JSON.stringify(propertyMatch)}; expected ${expectedMatchCountText(expectedMatchCount)}. Please narrow the search criteria`); |
The CCAPI provider stops after the first page of results. This may cause for example EC2 Prefix Lists that exist to not be found, if they don't occur in the first page of results. Make the provider retrieve all pages of results. Also in this PR: - New SDK mocks had been added without them being added to all the places where they needed to be added to be reset properly. Instead, put them all into an object so we can do a reliable `for` loop that will never go out of date again.
rix0rrr
force-pushed
the
huijbers/cc-provider-pagination
branch
from
b84bdfb to
d1b762d
Compare
mrgrain
approved these changes
Jul 2, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The CCAPI provider stops after the first page of results. This may cause for example EC2 Prefix Lists that exist to not be found, if they don't occur in the first page of results.
Make the provider retrieve all pages of results.
Also in this PR:
forloop that will never go out of date again.MaxResultsto 100, so we need to do fewer paginations and the chances of the error message finding too many elements has a reasonable indicator of the number of resources actually found.Fixes #587
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license