-
Notifications
You must be signed in to change notification settings - Fork 403
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
Allow specifying topic type for SNS module. #599
Allow specifying topic type for SNS module. #599
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stefanhorning Thank you for working on this. Could you add please some integration tests? In addition, a changelog fragment is also required https://docs.ansible.com/ansible/latest/dev_guide/testing/sanity/changelog.html.
@alinabuzachis done now. Let's see if all tests pass. |
Build fails in this line https://github.com/ansible/ansible-zuul-jobs/blob/master/roles/ansible-test/tasks/split_targets.yaml#L23, not sure how that's related to my change (if at all). |
it was probably more likely that this was the change that broke CI (20 hours ago) ansible/ansible-zuul-jobs@f8fa3e9#diff-1e727d74735315c5405dbbc015402f13da5e0c3d8d7a08cae148cc7dac1c4f56 |
Looks like the failure occurs when there's not matching tests. a |
Also just noticed that the tests for sns_topic are disabled anyway with alias |
There was a PR already to attempt enabling tests https://github.com/ansible-collections/community.aws/pull/519/files I will just do that in here now to see how it goes @tremble |
|
should read my own docs :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry about the delay here. The main change looks good.
2 minor things:
- defaults/main.yml in the test has a merge conflict (side effect of the zuul migration)
- shippable/aws/group1 is no longer needed in the aliases file, just remove the unsupported entry
@tremble merged in latest main now and resolved conflicts, or should I rather rebase? |
@stefanhorning this PR contains the following merge commits: Please rebase your branch to remove these commits. |
Please rebase if possible. With the migration to Zuul commits are now merged as-is rather than squashed. |
Add changelog fragment and extended integration test for sns_topic module
8e65c90
to
3f68feb
Compare
Yes, the bot already answered my question ;) Rebased now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apologies, one minor nit and we're good to merge.
Co-authored-by: Mark Chappell <mchappel@redhat.com>
done |
@stefanhorning Thanks for your contribution and patience. |
…type_param Allow specifying topic type for SNS module. SUMMARY Adding a topic_type param to SNS module (simillar to SQS). ISSUE TYPE Feature Pull Request COMPONENT NAME sns_topic module ADDITIONAL INFORMATION Simillar to SQS queues, SNS topics also allow specifying a type, that can either be 'Standard' or 'FIFO'. Furthermore if you have a FIFO SQS queue and want to subscribe that to an SNS topic, that one has to be FIFO too. Hence adding this flag is important and is actually just another option for the creat_topic action in Boto, see https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/sns.html#SNS.Client.create_topic Reviewed-by: Stefan Horning <None> Reviewed-by: Alina Buzachis <None> Reviewed-by: Mark Chappell <None>
…type_param Allow specifying topic type for SNS module. SUMMARY Adding a topic_type param to SNS module (simillar to SQS). ISSUE TYPE Feature Pull Request COMPONENT NAME sns_topic module ADDITIONAL INFORMATION Simillar to SQS queues, SNS topics also allow specifying a type, that can either be 'Standard' or 'FIFO'. Furthermore if you have a FIFO SQS queue and want to subscribe that to an SNS topic, that one has to be FIFO too. Hence adding this flag is important and is actually just another option for the creat_topic action in Boto, see https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/sns.html#SNS.Client.create_topic Reviewed-by: Stefan Horning <None> Reviewed-by: Alina Buzachis <None> Reviewed-by: Mark Chappell <None>
…ollections#599) Prepare for distutils.version being removed in Python 3.12 SUMMARY distutils has been deprecafed and will be removed from Python's stdlib in Python 3.12 (see https://python.org/dev/peps/pep-0632). This PR replaces the use of distutils.version.LooseVersion and distutils.version.StrictVersion with LooseVersion from the vendored copy of distutils.version included with ansible-core 2.12 (ansible/ansible#74644) if available, and falls back to distutils.version for ansible-core 2.11 and before. Since ansible-core 2.11 and earlier do not support Python 3.12 (since they use LooseVersion itself in various places), this incomplete fix should be OK for now. Also, the way this PR works (by adding a new module_utils version that abstracts away where LooseVersion comes from), it is easy to also fix this for ansible-core 2.11 and earlier later on. Signed-off-by: Abhijeet Kasurde akasurde@redhat.com ISSUE TYPE Bugfix Pull Request COMPONENT NAME changelogs/fragments/disutils.version.yml plugins/module_utils/core.py plugins/module_utils/version.py plugins/modules/ec2.py Reviewed-by: Felix Fontein <felix@fontein.de> Reviewed-by: Jill R <None> Reviewed-by: None <None>
SUMMARY
Adding a topic_type param to SNS module (simillar to SQS).
ISSUE TYPE
COMPONENT NAME
sns_topic module
ADDITIONAL INFORMATION
Simillar to SQS queues, SNS topics also allow specifying a type, that can either be 'Standard' or 'FIFO'. Furthermore if you have a FIFO SQS queue and want to subscribe that to an SNS topic, that one has to be FIFO too. Hence adding this flag is important and is actually just another option for the creat_topic action in Boto, see https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/sns.html#SNS.Client.create_topic