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

cloudwatchevent_rule: Fix json input handling for input_template #1883

Conversation

mandar242
Copy link
Contributor

@mandar242 mandar242 commented Nov 28, 2023

SUMMARY

Fixes #1842

This PR

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

cloudwatchevent_rule

ADDITIONAL INFORMATION

Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/213e45205d9744afa3ac5297e8cb3777

ansible-galaxy-importer RETRY_LIMIT in 6m 59s
✔️ build-ansible-collection SUCCESS in 16m 50s
✔️ ansible-test-splitter SUCCESS in 6m 06s
✔️ integration-amazon.aws-1 SUCCESS in 6m 18s
Skipped 43 jobs

@mandar242 mandar242 force-pushed the cloudwatchrule_event branch from 5626f68 to b9c18fb Compare November 28, 2023 21:03
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/c2cc9e8fac254858bfc61974f444855c

✔️ ansible-galaxy-importer SUCCESS in 4m 47s
✔️ build-ansible-collection SUCCESS in 15m 06s
✔️ ansible-test-splitter SUCCESS in 5m 35s
✔️ integration-amazon.aws-1 SUCCESS in 5m 39s
Skipped 43 jobs

Copy link
Contributor

@abikouo abikouo left a comment

Choose a reason for hiding this comment

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

Would be nice to add some unit tests for this update

@mandar242 mandar242 force-pushed the cloudwatchrule_event branch from 67baebc to 0580119 Compare December 4, 2023 10:13
@mandar242
Copy link
Contributor Author

Would be nice to add some unit tests for this update

there are integration tests for cloudwatchevent_rule, should we be adding more tests to existing integration tests or create new unit tests for this change?

Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/c41c0adb1d774868a76fc7b0bae0f582

✔️ ansible-galaxy-importer SUCCESS in 4m 39s
✔️ build-ansible-collection SUCCESS in 14m 41s
✔️ ansible-test-splitter SUCCESS in 5m 40s
✔️ integration-amazon.aws-1 SUCCESS in 6m 37s
✔️ integration-community.aws-1 SUCCESS in 22m 55s
✔️ integration-community.aws-2 SUCCESS in 37m 09s
✔️ integration-community.aws-3 SUCCESS in 46m 11s
✔️ integration-community.aws-4 SUCCESS in 16m 40s
✔️ integration-community.aws-5 SUCCESS in 18m 08s
✔️ integration-community.aws-6 SUCCESS in 14m 07s
✔️ integration-community.aws-7 SUCCESS in 19m 24s
✔️ integration-community.aws-8 SUCCESS in 7m 35s
✔️ integration-community.aws-9 SUCCESS in 15m 30s
✔️ integration-community.aws-10 SUCCESS in 22m 24s
✔️ integration-community.aws-11 SUCCESS in 9m 05s
✔️ integration-community.aws-12 SUCCESS in 18m 37s
integration-community.aws-13 FAILURE in 33m 09s
✔️ integration-community.aws-14 SUCCESS in 9m 30s
Skipped 29 jobs

Copy link
Contributor

@hakbailey hakbailey left a comment

Choose a reason for hiding this comment

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

I think it would be good to add both an integration test and an example in the module docs for a json input template.

plugins/modules/cloudwatchevent_rule.py Outdated Show resolved Hide resolved
@tremble
Copy link
Contributor

tremble commented Dec 6, 2023

Would be nice to add some unit tests for this update

there are integration tests for cloudwatchevent_rule, should we be adding more tests to existing integration tests or create new unit tests for this change?

Personally I'd stick to the integration tests for this change. The code is only consumed by this module, so adding unit tests is IMO "busy-work" that brings minimal value over the top of the integration tests. The only exception would be if they're handling hard to trigger edge cases.

@mandar242 mandar242 force-pushed the cloudwatchrule_event branch from 0580119 to b8d55ad Compare December 12, 2023 15:32
Copy link
Contributor

Build failed.
https://ansible.softwarefactory-project.io/zuul/buildset/dba09fb89dbd4362b12e45cdbc45403a

✔️ ansible-galaxy-importer SUCCESS in 4m 59s
✔️ build-ansible-collection SUCCESS in 16m 27s
✔️ ansible-test-splitter SUCCESS in 6m 37s
integration-amazon.aws-1 RETRY_LIMIT in 1m 45s
Skipped 43 jobs

@mandar242 mandar242 force-pushed the cloudwatchrule_event branch from b8d55ad to 90aeda0 Compare December 13, 2023 05:06
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/8c3faab57fb0457db4007d8509cb3f28

✔️ ansible-galaxy-importer SUCCESS in 4m 22s
✔️ build-ansible-collection SUCCESS in 15m 26s
✔️ ansible-test-splitter SUCCESS in 6m 00s
✔️ integration-amazon.aws-1 SUCCESS in 5m 45s
Skipped 43 jobs

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/cdedf8db1f2c4aa8b40b207a68a9bf87

✔️ ansible-galaxy-importer SUCCESS in 4m 50s
✔️ build-ansible-collection SUCCESS in 14m 01s
✔️ ansible-test-splitter SUCCESS in 5m 12s
✔️ integration-amazon.aws-1 SUCCESS in 8m 08s
Skipped 43 jobs

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/88e8206037b5407ca79b3e2496557b2f

✔️ ansible-galaxy-importer SUCCESS in 9m 21s
✔️ build-ansible-collection SUCCESS in 15m 20s
✔️ ansible-test-splitter SUCCESS in 6m 04s
✔️ integration-amazon.aws-1 SUCCESS in 6m 48s
Skipped 43 jobs

@mandar242 mandar242 force-pushed the cloudwatchrule_event branch from 11bbf5f to 8a226b5 Compare December 18, 2023 13:51
@mandar242 mandar242 added the mergeit Merge the PR (SoftwareFactory) label Dec 18, 2023
Copy link
Contributor

Build succeeded (gate pipeline).
https://ansible.softwarefactory-project.io/zuul/buildset/f3a86040db9f469583721fd64d48b225

✔️ ansible-galaxy-importer SUCCESS in 4m 51s
✔️ build-ansible-collection SUCCESS in 14m 38s
✔️ ansible-test-splitter SUCCESS in 6m 01s
✔️ integration-amazon.aws-1 SUCCESS in 9m 27s
Skipped 43 jobs

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 3b67513 into ansible-collections:main Dec 18, 2023
35 of 39 checks passed
@alalonde
Copy link

alalonde commented Feb 7, 2024

When will this be released?

@mandar242 mandar242 added the backport-7 PR should be backported to the stable-7 branch label Feb 8, 2024
Copy link

patchback bot commented Feb 8, 2024

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/3b67513e536c4e0d251b92b393d2ee4cecb4c86a/pr-1883

Backported as #1968

🤖 @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 Feb 8, 2024
cloudwatchevent_rule: Fix json input handling for input_template

SUMMARY

Fixes #1842
This PR

Moves _snakify() out of class CloudWatchEventRule https://github.com/ansible-collections/amazon.aws/pull/1883/files#diff-3a2f223edc4e34ea28b9859827a830844d92a8cd97f1300a2d16b1703a083bc8R213-R215

Fixes conditional in function _targets_to_put() to avoid wrong results due to _snakify() https://github.com/ansible-collections/amazon.aws/pull/1883/files#diff-3a2f223edc4e34ea28b9859827a830844d92a8cd97f1300a2d16b1703a083bc8R461-R463

Updates code to add quotes " to input_template only when the given input is not JSON to avoid quotes being added to provided JSON input https://github.com/ansible-collections/amazon.aws/pull/1883/files#diff-3a2f223edc4e34ea28b9859827a830844d92a8cd97f1300a2d16b1703a083bc8R452-R458

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

cloudwatchevent_rule
ADDITIONAL INFORMATION

Reviewed-by: Bikouo Aubin
Reviewed-by: Helen Bailey <[email protected]>
Reviewed-by: Alina Buzachis
Reviewed-by: Mandar Kulkarni <[email protected]>
(cherry picked from commit 3b67513)
@mandar242 mandar242 added the backport-6 PR should be backported to the stable-6 branch label Feb 8, 2024
Copy link

patchback bot commented Feb 8, 2024

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/3b67513e536c4e0d251b92b393d2ee4cecb4c86a/pr-1883

Backported as #1969

🤖 @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 Feb 8, 2024
cloudwatchevent_rule: Fix json input handling for input_template

SUMMARY

Fixes #1842
This PR

Moves _snakify() out of class CloudWatchEventRule https://github.com/ansible-collections/amazon.aws/pull/1883/files#diff-3a2f223edc4e34ea28b9859827a830844d92a8cd97f1300a2d16b1703a083bc8R213-R215

Fixes conditional in function _targets_to_put() to avoid wrong results due to _snakify() https://github.com/ansible-collections/amazon.aws/pull/1883/files#diff-3a2f223edc4e34ea28b9859827a830844d92a8cd97f1300a2d16b1703a083bc8R461-R463

Updates code to add quotes " to input_template only when the given input is not JSON to avoid quotes being added to provided JSON input https://github.com/ansible-collections/amazon.aws/pull/1883/files#diff-3a2f223edc4e34ea28b9859827a830844d92a8cd97f1300a2d16b1703a083bc8R452-R458

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME

cloudwatchevent_rule
ADDITIONAL INFORMATION

Reviewed-by: Bikouo Aubin
Reviewed-by: Helen Bailey <[email protected]>
Reviewed-by: Alina Buzachis
Reviewed-by: Mandar Kulkarni <[email protected]>
(cherry picked from commit 3b67513)
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Feb 13, 2024
…) (#1968)

[PR #1883/3b67513e backport][stable-7] cloudwatchevent_rule: Fix json input handling for input_template

This is a backport of PR #1883 as merged into main (3b67513).
SUMMARY

Fixes #1842
This PR


Moves _snakify() out of class CloudWatchEventRule https://github.com/ansible-collections/amazon.aws/pull/1883/files#diff-3a2f223edc4e34ea28b9859827a830844d92a8cd97f1300a2d16b1703a083bc8R213-R215


Fixes conditional in function _targets_to_put() to avoid wrong results due to _snakify() https://github.com/ansible-collections/amazon.aws/pull/1883/files#diff-3a2f223edc4e34ea28b9859827a830844d92a8cd97f1300a2d16b1703a083bc8R461-R463


Updates code to add quotes " to input_template only when the given input is not JSON to avoid quotes being added to provided JSON input https://github.com/ansible-collections/amazon.aws/pull/1883/files#diff-3a2f223edc4e34ea28b9859827a830844d92a8cd97f1300a2d16b1703a083bc8R452-R458



ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

cloudwatchevent_rule
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Feb 13, 2024
…) (#1969)

[PR #1883/3b67513e backport][stable-6] cloudwatchevent_rule: Fix json input handling for input_template

This is a backport of PR #1883 as merged into main (3b67513).
SUMMARY

Fixes #1842
This PR


Moves _snakify() out of class CloudWatchEventRule https://github.com/ansible-collections/amazon.aws/pull/1883/files#diff-3a2f223edc4e34ea28b9859827a830844d92a8cd97f1300a2d16b1703a083bc8R213-R215


Fixes conditional in function _targets_to_put() to avoid wrong results due to _snakify() https://github.com/ansible-collections/amazon.aws/pull/1883/files#diff-3a2f223edc4e34ea28b9859827a830844d92a8cd97f1300a2d16b1703a083bc8R461-R463


Updates code to add quotes " to input_template only when the given input is not JSON to avoid quotes being added to provided JSON input https://github.com/ansible-collections/amazon.aws/pull/1883/files#diff-3a2f223edc4e34ea28b9859827a830844d92a8cd97f1300a2d16b1703a083bc8R452-R458



ISSUE TYPE


Bugfix Pull Request

COMPONENT NAME

cloudwatchevent_rule
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis
@mandar242 mandar242 deleted the cloudwatchrule_event branch October 29, 2024 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-6 PR should be backported to the stable-6 branch backport-7 PR should be backported to the stable-7 branch mergeit Merge the PR (SoftwareFactory)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression: cloudwatchevent_rule doesn't properly support JSON input_templates
6 participants