Fix/aws rds refactor - #3069
Fix/aws rds refactor#3069centosinfra-prod-github-app[bot] merged 18 commits into
Conversation
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 12s |
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 3m 29s |
Addresses Alina's review comments on PR ansible-collections#3069: the create/delete/modify db_cluster_parameter_group boto3 calls now go through module_utils/_rds/api.py behind RDSErrorHandler, matching the existing convention for other RDS calls in this file, instead of using local try/except blocks. The parameter comparison loop is extracted into a local _get_changed_parameters() helper in the module.
|
Build failed. ❌ ansible-galaxy-importer POST_FAILURE in 8m 56s |
GomathiselviS
left a comment
GomathiselviS
left a comment
There was a problem hiding this comment.
Please run black and isort locally. This will fix the sorting and the trailing whitespace issues. (https://github.com/ansible-collections/cloud-content-handbook/blob/main/TeamPractices/Guidelines/coding_guidelines.md#automated-checks)
mandar242
left a comment
mandar242
left a comment
There was a problem hiding this comment.
Some of the commits look unsigned, other than that lgtm! Probably a rebase with signed commit would help
|
Build failed. ❌ ansible-galaxy-importer POST_FAILURE in 11m 42s |
This reverts commit 1a6db02.
Co-authored-by: Chyna Sanders <chynaesan@gmail.com>
Co-authored-by: Chyna Sanders <chynaesan@gmail.com>
Co-authored-by: Chyna Sanders <chynaesan@gmail.com>
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 2m 49s |
Addresses Alina's review comments on PR ansible-collections#3069: the create/delete/modify db_cluster_parameter_group boto3 calls now go through module_utils/_rds/api.py behind RDSErrorHandler, matching the existing convention for other RDS calls in this file, instead of using local try/except blocks. The parameter comparison loop is extracted into a local _get_changed_parameters() helper in the module.
Co-authored-by: GomathiselviS <gomathiselvi@gmail.com>
Co-authored-by: GomathiselviS <gomathiselvi@gmail.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
aca284a to
7cb49d5
Compare
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 05s |
Run black and isort on the RDS refactor files to address review feedback on import sorting and trailing whitespace. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Build succeeded. ✔️ ansible-galaxy-importer SUCCESS in 4m 43s |
Co-authored-by: GomathiselviS <gomathiselvi@gmail.com>
|
Build succeeded (gate pipeline). ✔️ ansible-galaxy-importer SUCCESS in 4m 01s |
8d634a0
into
ansible-collections:main
Docs Build 📝Thank you for contribution!✨ This PR has been merged and your docs changes will be incorporated when they are next published. |
Continues the migration started in ansible-collections#3069, following ansible-collections#2119 and ansible-collections#2138. Behaviour-preserving. - Normalize describe_db_cluster_parameter_groups() and describe_db_cluster_parameters() to the shared (client, **params) signature, dropping the unused `module` argument. - Drop the redundant botocore import and module.client() try/except, and catch AnsibleRDSError once at the module boundary in main(). - Split ensure_present() into smaller documented helpers, and reuse the group returned by the create call instead of describing it again. Both API calls return the same DBClusterParameterGroup shape, so output is unchanged. - Index the current parameters by name when comparing them against the requested ones. - Add unit tests for the shared helpers and both modules. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ies (#3089) SUMMARY Refactors rds_cluster_param_group onto the shared module_utils/rds utilities, continuing the migration started in #3069 and following the patterns established by #2119 (shared boto3 client helpers and error handling) and #2138 (shared describe_* functions). This is intended to be a pure refactor, no behaviour change. module_utils/_rds/api.py describe_db_cluster_parameter_groups() and describe_db_cluster_parameters() now take (client, **params) like every other describe_* helper in the module. Both previously took a module argument that became completely unused once #3069 replaced their manual try/except blocks with the @RDSErrorHandler.list_error_handler decorator. Filter construction moves to the callers, which is where the module params live. Renamed the connection argument to client in the cluster parameter group create/delete/modify helpers, for consistency with the rest of the file. rds_cluster_param_group Dropped the botocore import and the try/except around module.client(). AnsibleAWSModule already handles connection failures, so this was dead code. The client-level retry_decorator is kept, matching #3072. AnsibleRDSError raised by the shared helpers is now caught once at the module boundary in main(). Split ensure_present() into get_parameter_group(), create_parameter_group() and update_parameter_group(), with type hints and Google-style docstrings throughout. Reuse the parameter group returned by the create call instead of re-describing it, removing a redundant describe_db_cluster_parameter_groups() call. CreateDBClusterParameterGroup and DescribeDBClusterParameterGroups return the same DBClusterParameterGroup shape in the botocore service model, so the module output is unchanged. Index the current parameters by name when comparing them against the requested ones, instead of scanning the full list once per requested parameter. rds_cluster_param_group_info Adopted the new describe helper signatures and dropped the same redundant botocore import and connection try/except. ISSUE TYPE Refactoring Pull Request COMPONENT NAME rds_cluster_param_group rds_cluster_param_group_info module_utils/_rds/api.py ADDITIONAL INFORMATION Behaviour Everything is intended to be behaviour-preserving: the returned db_cluster_parameter_group keys, the parameter validation failure messages (Could not find parameter with name: ... and The parameter ... cannot be modified, still raised via module.fail_json() so they keep their exact text), the set of parameters sent to ModifyDBClusterParameterGroup, the tags: {} no-op, the check mode early exit and its message, idempotency, and the "does not exist" message on state=absent are all unchanged. The only externally observable difference is one fewer DescribeDBClusterParameterGroups call on the create path. Testing New unit tests: tests/unit/plugins/modules/test_rds_cluster_param_group.py (21 tests) and tests/unit/plugins/modules/test_rds_cluster_param_group_info.py (4 tests), plus 10 tests in test_rds_api.py covering the shared cluster parameter group helpers, which had no unit coverage before. Full RDS unit suite passes (852 tests). ansible-test sanity passes on all changed files. Existing integration tests under tests/integration/targets/rds_cluster_param_group are unchanged and each assertion was traced against the new code paths, but they have not been run against live AWS in this branch. A CI run would be good confirmation. Assisted-by: Claude Opus 5 Reviewed-by: Bianca Henderson <beeankha@gmail.com> Reviewed-by: Don Naro <dnaro@redhat.com>
7 participants
SUMMARY
Refactor
rds_cluster_param_groupmodule to align error handling with patterns established in #2119 and#2138. Part of #2003.
ISSUE TYPE
Refactoring Pull Request
COMPONENT NAME
rds_cluster_param_groupmodule_utils/_rds/api.pymodule_utils/_rds/common.pyADDITIONAL INFORMATION
module_utils/_rds/common.py:
module_utils/_rds/api.py:
describe_db_cluster_parameter_groups()to use@RDSErrorHandler.list_error_handlerdecoratorinstead of manual
try/exceptwithis_boto3_error_codedescribe_db_cluster_parameters()to use@RDSErrorHandler.list_error_handler decoratorinstead of manual
try/exceptwith is_boto3_error_coderds_cluster_param_group module:
modify_parameters(),ensure_present(), andensure_absent()functionsAssisted by Claude opus 4.6