New Resources: Notification Hubs #1589
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
``` $ acctests azurerm TestAccAzureRMNotificationHubNamespace_ === RUN TestAccAzureRMNotificationHubNamespace_importFree --- PASS: TestAccAzureRMNotificationHubNamespace_importFree (134.35s) === RUN TestAccAzureRMNotificationHubNamespace_free --- PASS: TestAccAzureRMNotificationHubNamespace_free (126.57s) === RUN TestAccAzureRMNotificationHubNamespace_updateSku --- PASS: TestAccAzureRMNotificationHubNamespace_updateSku (143.16s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 404.111s ```
``` $ acctests azurerm TestAccDataSourceAzureRMNotificationHubNamespace_free === RUN TestAccDataSourceAzureRMNotificationHubNamespace_free --- PASS: TestAccDataSourceAzureRMNotificationHubNamespace_free (130.03s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 130.081s ```
tombuildsstuff
force-pushed
the
notification-hubs
branch
from
d1c720b to
7c8f0f6
Compare
tombuildsstuff
changed the title
``` $ acctests azurerm TestAccAzureRMNotificationHub_basic === RUN TestAccAzureRMNotificationHub_basic --- PASS: TestAccAzureRMNotificationHub_basic (284.22s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 284.273s ```
Renaming the fields to better match the portal/terms used in Apple/Google Dev Portals
Tests pass: ``` ```
tombuildsstuff
changed the title
tombuildsstuff
force-pushed
the
notification-hubs
branch
from
59ca5de to
3914a28
Compare
Tests pass: ``` $ acctests azurerm TestAccAzureRMNotificationHubNamespace_ === RUN TestAccAzureRMNotificationHubNamespace_importFree --- PASS: TestAccAzureRMNotificationHubNamespace_importFree (71.04s) === RUN TestAccAzureRMNotificationHubNamespace_free --- PASS: TestAccAzureRMNotificationHubNamespace_free (73.17s) === RUN TestAccAzureRMNotificationHubNamespace_updateSku --- PASS: TestAccAzureRMNotificationHubNamespace_updateSku (86.43s) PASS ok github.com/terraform-providers/terraform-provider-azurerm/azurerm 230.677s ```
|
Data Source tests pass: |
``` azurerm_notification_hub_namespace.test: Error creating/updating Notification Hub Namesapce "acctestnhn-4049058900878420170" (Resource Group "acctestrg-4049058900878420170"): notificationhubs.NamespacesClient#CreateOrUpdate: Failure sending request: StatusCode=409 -- Original Error: autorest/azure: Service returned an error. Status=<nil> Code="Conflict" Message="Namespace acctestnhn-4049058900878420170 is in state Created. Cannot update the property" ```
tombuildsstuff
commented
Jul 18, 2018
| } | ||
|
|
||
| // try triggering a deleting again | ||
| client.Delete(ctx, resourceGroupName, name) |
There was a problem hiding this comment.
just want to call this out as this looks odd on first glance - the Namespace delete method doesn't actually delete it some of the time, but repeatedly hitting it seems to work so 🙃
|
Resource tests pass:
|
mbfrahry
approved these changes
Jul 19, 2018
| token := input["token"].(string) | ||
|
|
||
| applicationEndpoints := map[string]string{ | ||
| "Production": "https://api.push.apple.com:443/3/device", |
There was a problem hiding this comment.
Thoughts on making the endpoints constants?
|
|
||
| if endpoint := input.Endpoint; endpoint != nil { | ||
| applicationEndpoints := map[string]string{ | ||
| "https://api.push.apple.com:443/3/device": "Production", |
There was a problem hiding this comment.
Thoughts on making the endpoints constants?
| Importer: &schema.ResourceImporter{ | ||
| State: schema.ImportStatePassthrough, | ||
| }, | ||
| // TODO: customizeDiff for send+listen when manage selected |
There was a problem hiding this comment.
Just confirming that this is for later down the road
There was a problem hiding this comment.
yep, it's documented so this isn't necessary at this time
| return fmt.Errorf("Error waiting for Notification Hub %q (Resource Group %q) to be deleted: %s", name, resourceGroup, err) | ||
| } | ||
|
|
||
| err = future.WaitForCompletionRef(ctx, client.Client) |
There was a problem hiding this comment.
Do we want to comment this out until the issue mentioned above is fixed? Or is it fine to wait for when the namespace is deleted?
There was a problem hiding this comment.
🤦♂️ I thought I'd done this, but yes you're right this should be removed
tombuildsstuff
force-pushed
the
notification-hubs
branch
from
51e72c4 to
5343ad5
Compare
|
Data Source tests still pass: |
|
Resource tests pass:
|
|
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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I went to start work on this and it turns out apparently I started this in April ¯_(ツ)_/¯
Notification Hub Namespaces
Notification Hub
Since this requires valid APNS and GCM credentials to test - I'll have to assert these manually for the moment; so the Acceptance Tests will be limited to the core Notification Hub workflow.
Notification Hub Authorization Rule
Other