route53_info - fix max_items when not paginating #1384
Conversation
ansibullbot
added
bug
|
Hi @mcoot, Thanks for taking the time to submit this PR. We have integration tests in |
|
No worries - I was considering doing so initially, but it didn't obviously fit into the existing flow (and there were no existing integration tests using the relevant operations). I've added some steps here to try one of the non-paginated operations and assert on the result (which also tests for regression against what #813 aimed to fix) – albeit I don't have a setup for running these against my own account so I'm relying on the CI run here to check the test is correct. |
|
Unfortunately, some of our older (and often more complex) modules have very patchy integration test coverage. While "complete" coverage would be nice, slowly targetting 'known' bugs for additional tests is better than nothing. |
ansibullbot
added
integration
|
Well good thing I did add the test, seeing as it failed. Fix up shortly. The error is a little surprising - |
tremble
added
backport-3
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
|
Prior to #813 the input parameter didn't have a type defined, this was updated in #813 to require that an integer was passed ( |
|
Ah, IAM this time: Hmm, how are the IAM perms for the integration tests configured here? |
|
Since I'm in the process of preparing the next minor release (and it'd be good to get this added), I've tested this change locally, I'm going to comment out the integration test and follow up with the relevant folks to get the tests and CI permissions updated. |
tremble
added
mergeit
Backport to stable-3: 💚 backport PR created✅ Backport PR branch: Backported as #1387 🤖 @patchback |
route53_info - fix max_items when not paginating SUMMARY As reported in #1383, the route53_info module presently fails to run with a boto3 parameter validation error if run with particular combinations of parameters, specifically: query: hosted_zone parameter with hosted_zone_method: list_by_name query: reusable_delegation_set without specifying a delegation_set_id I believe this is a regression introduced in #813 ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info ADDITIONAL INFORMATION Some further information is described in the issue but tl;dr the prior PR converted all cases in the module where params['MaxItems'] was set to instead pass a params['PaginationConfig'], however this should only be done if a boto3 paginator is actually being used, and will fail (as noted above, due to parameter validation) if called with a regular boto3 client method. Hence this PR switches back to directly setting MaxItems on the methods that do not use a paginator. Reviewed-by: Mark Chappell <None> (cherry picked from commit 569fff4)
Backport to stable-4: 💚 backport PR created✅ Backport PR branch: Backported as #1388 🤖 @patchback |
route53_info - fix max_items when not paginating SUMMARY As reported in #1383, the route53_info module presently fails to run with a boto3 parameter validation error if run with particular combinations of parameters, specifically: query: hosted_zone parameter with hosted_zone_method: list_by_name query: reusable_delegation_set without specifying a delegation_set_id I believe this is a regression introduced in #813 ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info ADDITIONAL INFORMATION Some further information is described in the issue but tl;dr the prior PR converted all cases in the module where params['MaxItems'] was set to instead pass a params['PaginationConfig'], however this should only be done if a boto3 paginator is actually being used, and will fail (as noted above, due to parameter validation) if called with a regular boto3 client method. Hence this PR switches back to directly setting MaxItems on the methods that do not use a paginator. Reviewed-by: Mark Chappell <None> (cherry picked from commit 569fff4)
|
Thanks @mcoot, Hopefully we can get this out in a release in the next week or so. |
[PR #1384/569fff4c backport][stable-3] route53_info - fix max_items when not paginating This is a backport of PR #1384 as merged into main (569fff4). SUMMARY As reported in #1383, the route53_info module presently fails to run with a boto3 parameter validation error if run with particular combinations of parameters, specifically: query: hosted_zone parameter with hosted_zone_method: list_by_name query: reusable_delegation_set without specifying a delegation_set_id I believe this is a regression introduced in #813 ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info ADDITIONAL INFORMATION Some further information is described in the issue but tl;dr the prior PR converted all cases in the module where params['MaxItems'] was set to instead pass a params['PaginationConfig'], however this should only be done if a boto3 paginator is actually being used, and will fail (as noted above, due to parameter validation) if called with a regular boto3 client method. Hence this PR switches back to directly setting MaxItems on the methods that do not use a paginator. Reviewed-by: Mark Chappell <None>
[PR #1384/569fff4c backport][stable-4] route53_info - fix max_items when not paginating This is a backport of PR #1384 as merged into main (569fff4). SUMMARY As reported in #1383, the route53_info module presently fails to run with a boto3 parameter validation error if run with particular combinations of parameters, specifically: query: hosted_zone parameter with hosted_zone_method: list_by_name query: reusable_delegation_set without specifying a delegation_set_id I believe this is a regression introduced in #813 ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info ADDITIONAL INFORMATION Some further information is described in the issue but tl;dr the prior PR converted all cases in the module where params['MaxItems'] was set to instead pass a params['PaginationConfig'], however this should only be done if a boto3 paginator is actually being used, and will fail (as noted above, due to parameter validation) if called with a regular boto3 client method. Hence this PR switches back to directly setting MaxItems on the methods that do not use a paginator. Reviewed-by: Mark Chappell <None>
|
Awesome, thanks for your help with this! |
route53_info - enable max_items tests SUMMARY Enables the tests from #1384 ISSUE TYPE Feature Pull Request COMPONENT NAME route53_info ADDITIONAL INFORMATION
…#1384) route53_info - fix max_items when not paginating SUMMARY As reported in ansible-collections#1383, the route53_info module presently fails to run with a boto3 parameter validation error if run with particular combinations of parameters, specifically: query: hosted_zone parameter with hosted_zone_method: list_by_name query: reusable_delegation_set without specifying a delegation_set_id I believe this is a regression introduced in ansible-collections#813 ISSUE TYPE Bugfix Pull Request COMPONENT NAME route53_info ADDITIONAL INFORMATION Some further information is described in the issue but tl;dr the prior PR converted all cases in the module where params['MaxItems'] was set to instead pass a params['PaginationConfig'], however this should only be done if a boto3 paginator is actually being used, and will fail (as noted above, due to parameter validation) if called with a regular boto3 client method. Hence this PR switches back to directly setting MaxItems on the methods that do not use a paginator. Reviewed-by: Mark Chappell <None> This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections@569fff4
route53_info - enable max_items tests SUMMARY Enables the tests from ansible-collections#1384 ISSUE TYPE Feature Pull Request COMPONENT NAME route53_info ADDITIONAL INFORMATION This commit was initially merged in https://github.com/ansible-collections/community.aws See: ansible-collections@7427a0d
Allow to disable encryption on cloudtrail SUMMARY Allow to disable encryption on cloudtrail. Note: Tests are not run in CI because of missing policy. ISSUE TYPE Bugfix Pull Request Docs Pull Request Feature Pull Request New Module Pull Request COMPONENT NAME ADDITIONAL INFORMATION Reviewed-by: Mark Chappell
SUMMARY
As reported in #1383, the
route53_infomodule presently fails to run with a boto3 parameter validation error if run with particular combinations of parameters, specifically:query: hosted_zone parameterwithhosted_zone_method: list_by_namequery: reusable_delegation_setwithout specifying adelegation_set_idI believe this is a regression introduced in #813
ISSUE TYPE
COMPONENT NAME
route53_info
ADDITIONAL INFORMATION
Some further information is described in the issue but tl;dr the prior PR converted all cases in the module where
params['MaxItems']was set to instead pass aparams['PaginationConfig'], however this should only be done if a boto3 paginator is actually being used, and will fail (as noted above, due to parameter validation) if called with a regular boto3 client method.Hence this PR switches back to directly setting MaxItems on the methods that do not use a paginator.