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 Platform Application Support #8294

Closed

Conversation

megaqube
Copy link

Based on @dalehamel's original pull request implementation for SNS Application Support.

Added Import support.

Tested for GCM and APNS platform types. Unit tests added with API Credentials extracted out to Environment variables.

Website Documentation updated.

@megaqube
Copy link
Author

Currently tests pull GCM and APNS_SANDBOX credentials from the Environment so that I can run these tests locally without accidentally committing them, but Travis CI test are failing due to this.

@radeksimko
Copy link
Member

@Xirel To answer your question in
#3335 (comment)

Environment variable is a good approach, we can set those as part of the build in Travis for nightly runs. That said os.Exit is almost never a good thing when used in tests. Have a look at how we deal with the same issue in aws_vpc_peering_connection 😉

@megaqube
Copy link
Author

@radeksimko Thanks for pointing out the example. I've made the updates to the SNS_APPLICATION tests.

@megaqube
Copy link
Author

@radeksimko - any other suggestions on what I need in order to get this feature merge in?

@iceycake
Copy link
Contributor

Any update on this? I need this feature.

@bigkraig
Copy link
Contributor

@radeksimko @stack72 dudes! @Xirel fixed up the os.Exit, is it good to merge?

@mikecook mikecook force-pushed the SNS_PLATFORM_APPLICATION branch 2 times, most recently from 13223bf to 86d7ad4 Compare September 22, 2016 23:13
@mikecook
Copy link

@radeksimko @stack72 Fixes made, anything else needed for merge?

@megaqube megaqube mentioned this pull request Oct 13, 2016
3 tasks
@bigkraig
Copy link
Contributor

bigkraig commented Nov 3, 2016

@radeksimko @stack72 any help?

@mikecook
Copy link

@clstokes This is the PR that needs some love. Thanks!

@jedi4ever
Copy link

👍 for merging this in

@megaqube
Copy link
Author

Pretty please... 🙏

@bigkraig
Copy link
Contributor

@radeksimko @stack72 👀

@iceycake
Copy link
Contributor

Waiting for a long time. Can this PR have some love?

@jdori
Copy link

jdori commented Dec 2, 2016

I'm also waiting on this PR.

@danscan
Copy link

danscan commented Dec 6, 2016

Very excited to see this PR merged! @stack72, can you please review this? 😁

@rokka-n
Copy link

rokka-n commented Dec 8, 2016

Lets do it!

@danscan
Copy link

danscan commented Dec 9, 2016

This seems to have slipped through the cracks! Hey, @mitchellh could you please merge or give some quick feedback? :D

@stack72
Copy link
Contributor

stack72 commented Dec 9, 2016

Hi all

this hasn't slipped through the cracks by any means - I promise we will get to this soon. We need to get the test accounts in place to set this up as part of our nightly tests before we merge it. This is on my to-do list to pick up :)

Paul

@megaqube
Copy link
Author

megaqube commented Dec 9, 2016

👍

@mikecook mikecook force-pushed the SNS_PLATFORM_APPLICATION branch from 86d7ad4 to 644022d Compare December 13, 2016 23:57
@mikecook
Copy link

Rebased on new master to resolve conflict.

@rokka-n
Copy link

rokka-n commented Jan 16, 2017

Can somebody give it a green light, please

@mikecook mikecook force-pushed the SNS_PLATFORM_APPLICATION branch from 644022d to 3d730e1 Compare January 19, 2017 03:04
@ThatsMrTalbot
Copy link

Is this still held up by test accounts?

@mikecook
Copy link

@stack72 @mitchellh @radeksimko @clstokes This PR just had its 6 month birthday on the 18th. It would love to be merged. Pretty please?

@mikecook
Copy link

@stack72 Is this still held up by test accounts?

@tazjin
Copy link

tazjin commented Mar 23, 2017

tumbleweed

@mikecook mikecook force-pushed the SNS_PLATFORM_APPLICATION branch from 3d730e1 to 8336b8b Compare April 10, 2017 23:00
@tartakovsky
Copy link

That's the only thing stopping us from completely relying on terraform for our AWS infrastructure deployment at the moment.

@stack72 guys, there are real production users and huge systems waiting for that for half a year already. Can you let us know what's preventing this from being merged? Can we do something to speed up the process?

@bigkraig
Copy link
Contributor

bigkraig commented Jul 7, 2017

⚰️

@kwent
Copy link

kwent commented Jul 13, 2017

Let's do it. How can i help ?

@grubernaut
Copy link
Contributor

Hello all,

First of all, I'm extremely sorry that this PR has sat here for so long without response, review, or resolve. This is absolutely our fault, and definitely not something that I'm proud of. We absolutely need to be better at reviewing and managing pull requests from outside contributors. Ignoring the obvious issue we have in this area will only serve to injure the amazing and tremendous open source community we already have.

Part of the provider-level split out will also help us out in this area. With only 6 full-time employees working on Terraform Providers, a lot of issues and PR's had the habit of "slipping through the cracks". With the split-out, we're able to give a finer granularity of permissions to outside contributors, enabling outside maintainers with full commit-rights on provider repositories that they manage. This incredible help from our talented community is going to be essential moving forwards, and we couldn't do it without all your help.

That being said, it looks like some tremendous work has already been done to migrate this PR to the new aws provider. Thus, I'm going to close this PR and I'm currently reviewing the new PR here: hashicorp/terraform-provider-aws#1101.

I sincerely thank you all for your patience, continued support, and understanding for the past year that this has been open.

@grubernaut grubernaut closed this Jul 13, 2017
@ghost
Copy link

ghost commented Apr 8, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.