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

sns_topic: Allow canonical international-format phone numbers in SMS subscriptions #454

Merged
merged 4 commits into from
Mar 8, 2021

Conversation

overhacked
Copy link
Contributor

SUMMARY

Adds + to the list of acceptable characters in an SMS endpoint.
Fixes #453.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

sns_topic

Adds `+` to the list of acceptable characters in an SMS endpoint.

Closes ansible-collections#453.
@overhacked overhacked changed the title Update sns_topic.py sns_topic: Allow canonical international-format phone numbers in SMS subscriptions Mar 3, 2021
@ansibullbot
Copy link

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review module module needs_triage new_contributor Help guide this first time contributor plugins plugin (any type) small_patch Hopefully easy to review labels Mar 3, 2021
@alinabuzachis
Copy link
Contributor

Hi @overhacked, thank you for your contribution. May you kindly add a changelog https://docs.ansible.com/ansible/latest/community/development_process.html#changelogs?
Otherwise, LGTM.

Add comment documenting to what standard SMS endpoint addresses (phone
numbers) are canonicalized
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.

Just a tiny niggle

@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 labels Mar 8, 2021
* Get quoting correct
* Simplify message to leave details in PR description

Co-authored-by: Mark Chappell <[email protected]>
@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 Mar 8, 2021
@tremble
Copy link
Contributor

tremble commented Mar 8, 2021

Thanks for your submission

@tremble tremble merged commit 36a2062 into ansible-collections:main Mar 8, 2021
danquixote pushed a commit to danquixote/community.aws that referenced this pull request May 16, 2021
…subscriptions (ansible-collections#454)

* Update sns_topic.py

Adds `+` to the list of acceptable characters in an SMS endpoint.

Closes ansible-collections#453.

* Add changelog fragment for ansible-collections#454

* sns_topic: comment explaining SMS canonicalization

Add comment documenting to what standard SMS endpoint addresses (phone
numbers) are canonicalized

* sns_topic: fix changelog

* Get quoting correct
* Simplify message to leave details in PR description

Co-authored-by: Mark Chappell <[email protected]>

Co-authored-by: Mark Chappell <[email protected]>
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
…subscriptions (ansible-collections#454)

* Update sns_topic.py

Adds `+` to the list of acceptable characters in an SMS endpoint.

Closes ansible-collections#453.

* Add changelog fragment for ansible-collections#454

* sns_topic: comment explaining SMS canonicalization

Add comment documenting to what standard SMS endpoint addresses (phone
numbers) are canonicalized

* sns_topic: fix changelog

* Get quoting correct
* Simplify message to leave details in PR description

Co-authored-by: Mark Chappell <[email protected]>

Co-authored-by: Mark Chappell <[email protected]>
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
…subscriptions (ansible-collections#454)

* Update sns_topic.py

Adds `+` to the list of acceptable characters in an SMS endpoint.

Closes ansible-collections#453.

* Add changelog fragment for ansible-collections#454

* sns_topic: comment explaining SMS canonicalization

Add comment documenting to what standard SMS endpoint addresses (phone
numbers) are canonicalized

* sns_topic: fix changelog

* Get quoting correct
* Simplify message to leave details in PR description

Co-authored-by: Mark Chappell <[email protected]>

Co-authored-by: Mark Chappell <[email protected]>
danielcotton pushed a commit to danielcotton/community.aws that referenced this pull request Nov 23, 2021
…subscriptions (ansible-collections#454)

* Update sns_topic.py

Adds `+` to the list of acceptable characters in an SMS endpoint.

Closes ansible-collections#453.

* Add changelog fragment for ansible-collections#454

* sns_topic: comment explaining SMS canonicalization

Add comment documenting to what standard SMS endpoint addresses (phone
numbers) are canonicalized

* sns_topic: fix changelog

* Get quoting correct
* Simplify message to leave details in PR description

Co-authored-by: Mark Chappell <[email protected]>

Co-authored-by: Mark Chappell <[email protected]>
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
…>= 1.17.18 (ansible-collections#454)

Don't check aws_az_info 'zone_type' it's only returned when botocore >= 1.17.18

SUMMARY
AWS 'zone_type' info is only returned when botocore >= 1.17.18.  Since we don't document what's returned we can get away with not testing this until the next botocore bump.
ISSUE TYPE

Tests Pull Request

COMPONENT NAME
aws_az_info
ADDITIONAL INFORMATION
Depends-On: ansible-collections#460

Reviewed-by: Alina Buzachis <None>
Reviewed-by: None <None>
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request May 25, 2022
…nsible-collections#404)

Add constraints.txt and requirements.txt for unit/integration tests

SUMMARY
Now that we state that we support specific minimum versions of the AWS SDKs, make sure we base our unit/integration tests against them such that modules need to explicitly test/request newer versions of the SDKs.
ISSUE TYPE

Feature Pull Request

COMPONENT NAME
tests/integration
tests/unit
ADDITIONAL INFORMATION
Once merged into amazon.aws we should merge this into community.aws
Depends-On: ansible-collections#453
Depends-On: ansible-collections#454
Depends-On: ansible-collections#450
Depends-On: ansible-collections#496
See also: ansible/ansible-zuul-jobs#991

Reviewed-by: Jill R <None>
Reviewed-by: None <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug community_review module module needs_triage new_contributor Help guide this first time contributor plugins plugin (any type) small_patch Hopefully easy to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sns_topic module always deletes and replaces SMS subscriptions
4 participants