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

Port the sns module to boto3 #45634

Merged
merged 3 commits into from
Oct 7, 2018
Merged

Port the sns module to boto3 #45634

merged 3 commits into from
Oct 7, 2018

Conversation

flowerysong
Copy link
Contributor

SUMMARY

Ports the sns module to boto3 and AnsibleAWSModule. Adds support for protocol-specific messages targeting newer protocols, including 'lambda'.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

sns

ANSIBLE VERSION

2.8

ADDITIONAL INFORMATION
ok: [tasty-deepone.syslog.x.mail.umich.edu -> localhost] => {
    "changed": false, 
    "invocation": {
        "module_args": {
            "application": null, 
            "aws_access_key": null, 
            "aws_secret_key": null, 
            "ec2_url": null, 
            "email": null, 
            "email_json": null, 
            "http": null, 
            "https": null, 
            "lambda": "{\"source\": \"pandora.x.mail.umich.edu\", \"message\": \"H\\u00e9r fylgir stu\\u00f0ning hverri sannkenning, sv\\u00e1 sem k\\u00f6llu\\u00f0 er eggin \\u00f3h\\u00f6r\\u00f0, en fullhvatir mennirnir.\", \"sourcetype\": \"ansible\"}", 
            "message_attributes": null, 
            "message_structure": "json", 
            "msg": "Hér er stafasetning sú, er hætti ræðr ok kveðandi gerir.", 
            "profile": "vdc-nonprod", 
            "region": "us-west-2", 
            "security_token": null, 
            "sms": null, 
            "sqs": null, 
            "subject": "Hello from Ansible", 
            "topic": "rootmail", 
            "validate_certs": true
        }
    }, 
    "message_id": "d46335db-7f32-5843-a5f2-61cfbf9eb7b5", 
    "msg": "OK"
}

@ansibot
Copy link
Contributor

ansibot commented Sep 13, 2018

@ansibot
Copy link
Contributor

ansibot commented Sep 13, 2018

Hi @flowerysong,

Thank you for the pullrequest, just so you are aware we have a dedicated Working Group for aws.
You can find other people interested in this in #ansible-aws on Freenode IRC
For more information about communities, meetings and agendas see https://github.com/ansible/community

click here for bot help

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 aws cloud community_review In order to be merged, this PR must follow the community review workflow. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. labels Sep 13, 2018
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Sep 13, 2018
@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Sep 13, 2018
lib/ansible/modules/cloud/amazon/sns.py Outdated Show resolved Hide resolved
lib/ansible/modules/cloud/amazon/sns.py Outdated Show resolved Hide resolved
@willthames
Copy link
Contributor

This is great work, thanks. Could we have a test suite too please.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Sep 13, 2018
Copy link
Contributor

@s-hertel s-hertel left a comment

Choose a reason for hiding this comment

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

Looks good. Just could use some integration tests to help ensure backwards compatibility.

return topic[u'TopicArn']

paginator = client.get_paginator('list_topics')
topic_iterator = paginator.paginate()
Copy link
Contributor

Choose a reason for hiding this comment

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

Please wrap the client and paginator methods in a try/except and handle BotoCoreError and ClientError

@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Sep 26, 2018
@ansibot ansibot removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Sep 28, 2018
@ryansb
Copy link
Contributor

ryansb commented Oct 5, 2018

@willthames can you have a second look at this?

@willthames
Copy link
Contributor

rebuild_merge

@flowerysong flowerysong closed this Oct 7, 2018
@flowerysong flowerysong reopened this Oct 7, 2018
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Oct 7, 2018
@ansibot ansibot merged commit be05069 into ansible:devel Oct 7, 2018
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.8 This issue/PR affects Ansible v2.8 aws cloud core_review In order to be merged, this PR must follow the core review workflow. feature This issue/PR relates to a feature request. module This issue/PR relates to a module. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants