Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

route53_info: Add snake_cased return key,values to remaining methods #1322

Conversation

mandar242
Copy link
Contributor

SUMMARY

Following up on #1236
Found more places where route53_info module does not return a snake_case output.

Added snake_case output to checker_ip_range_details , reusable_delegation_set_details, and get_health_check methods.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

route53_info

@ansibullbot
Copy link

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review module module needs_triage plugins plugin (any type) labels Jul 6, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 4m 16s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 01s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 49s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 38s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 9m 13s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 10m 53s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 37s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 6m 33s
✔️ ansible-test-splitter SUCCESS in 2m 37s
✔️ integration-community.aws-1 SUCCESS in 8m 10s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

Copy link
Contributor

@jatorcasso jatorcasso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The old keys should still be returned in addition to the new ones you added (with a deprecation message etc etc). Also, it has to be muuuch easier to simply camel_dict_to_snake_dict the final results prior to module.exit_json, rather than going through and modifying each individual key thats CamelCased. That's why we have module utils ;)

@mandar242
Copy link
Contributor Author

The old keys should still be returned in addition to the new ones you added (with a deprecation message etc etc). Also, it has to be muuuch easier to simply camel_dict_to_snake_dict the final results prior to module.exit_json, rather than going through and modifying each individual key thats CamelCased. That's why we have module utils ;)

Yes, the old keys(camel_cased) are still being returned along with the new (snake_cased) keys.
for example

{
    "changed": false,
    "failed": false
    "ResponseMetadata": {
        "HTTPHeaders": {
            "content-length": "1189",
            "content-type": "text/xml",
            "date": "Thu, 07 Jul 2022 02:53:59 GMT",
            "x-amzn-requestid": "1234567890xxxxxxxx"
        },
        "HTTPStatusCode": 200,
        "RequestId": "1234567890xxxxxxxx",
        "RetryAttempts": 0
    },
    "CheckerIpRanges": [
        "15.177.2.0/23",
        ...
        ...
    ],
    "checker_ip_ranges": [
        "15.177.2.0/23",
        ...
        ...
    ],
}

also yes, using camel_dict_to_snake_dict before module.exit_json itself could have been a better approach but considering how the module is structured, especially at https://github.com/ansible-collections/community.aws/blob/main/plugins/modules/route53_info.py#L693-L700, it was cleaner way to implement camel_dict_to_snake_dict the way it is now I guess.

Added the deprecation warnings.

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 3m 56s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 19s
✔️ ansible-test-sanity-docker-devel SUCCESS in 12m 03s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 05s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 9m 07s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 12m 30s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 7m 30s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 6m 23s
✔️ ansible-test-splitter SUCCESS in 2m 32s
✔️ integration-community.aws-1 SUCCESS in 7m 54s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@tremble tremble added backport-3 PR should be backported to the stable-3 branch backport-4 PR should be backported to the stable-4 branch labels Jul 7, 2022
Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mandar242,

Since the original PR (#1236) already got released we need a changelog to go with this.
Additionally I don't see any return value documentation, and some integration tests would also be good...

@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed community_review needs_triage labels Jul 7, 2022
@github-actions
Copy link

github-actions bot commented Jul 7, 2022

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

@ansibullbot ansibullbot added community_review and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jul 7, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 3m 52s
✔️ build-ansible-collection SUCCESS in 4m 51s
✔️ ansible-test-sanity-docker-devel SUCCESS in 11m 44s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 38s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 54s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 10m 23s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 09s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 6m 02s
✔️ ansible-test-splitter SUCCESS in 3m 21s
✔️ integration-community.aws-1 SUCCESS in 7m 36s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@tremble tremble added mergeit Merge the PR (SoftwareFactory) and removed backport-3 PR should be backported to the stable-3 branch labels Jul 7, 2022
@tremble
Copy link
Contributor

tremble commented Jul 7, 2022

regate

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

✔️ ansible-galaxy-importer SUCCESS in 4m 11s
✔️ build-ansible-collection SUCCESS in 4m 48s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 30s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 43s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 20s
✔️ ansible-test-sanity-docker-stable-2.13 SUCCESS in 11m 49s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 29s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 6m 43s
✔️ ansible-test-splitter SUCCESS in 2m 32s
✔️ integration-community.aws-1 SUCCESS in 8m 07s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit d7f3862 into ansible-collections:main Jul 7, 2022
@patchback
Copy link

patchback bot commented Jul 7, 2022

Backport to stable-4: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-4/d7f38627fdb9e8309a6fcd7229610dc3f3da584e/pr-1322

Backported as #1327

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jul 7, 2022
…1322)

route53_info: Add snake_cased return key,values to remaining methods

SUMMARY

Following up on #1236
Found more places where route53_info module does not return a snake_case output.
Added snake_case output to checker_ip_range_details , reusable_delegation_set_details, and get_health_check methods.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

route53_info

Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Mark Chappell <None>
(cherry picked from commit d7f3862)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Jul 8, 2022
…1322) (#1327)

[PR #1322/d7f38627 backport][stable-4] route53_info: Add snake_cased return key,values to remaining methods

This is a backport of PR #1322 as merged into main (d7f3862).
SUMMARY

Following up on #1236
Found more places where route53_info module does not return a snake_case output.
Added snake_case output to checker_ip_range_details , reusable_delegation_set_details, and get_health_check methods.

ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

route53_info

Reviewed-by: Mark Chappell <None>
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
…nsible-collections#1322)

route53_info: Add snake_cased return key,values to remaining methods

SUMMARY

Following up on ansible-collections#1236
Found more places where route53_info module does not return a snake_case output.
Added snake_case output to checker_ip_range_details , reusable_delegation_set_details, and get_health_check methods.

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

route53_info

Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Mark Chappell <None>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@d7f3862
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
ec2_snapshot - stability - relax integration tests

SUMMARY
When using pagination, you're not guaranteed to get "max_items", it's a maximum.  Sometimes you'll get less.  Relax the ec2_snapshot tests a little to take this into account.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
ec2_snapshot
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
Bulk update imports after reshuffle

Depends-On: ansible-collections#1323
Depends-On: ansible-collections#1322
Depends-On: ansible-collections#1321
SUMMARY
module_utils code got split up.  Bulk update the imports to reflect this
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
various
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-4 PR should be backported to the stable-4 branch bug This issue/PR relates to a bug community_review mergeit Merge the PR (SoftwareFactory) module module plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants