-
Notifications
You must be signed in to change notification settings - Fork 9.5k
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
provider/azurerm: Event Hub Namespaces #9297
Conversation
448c6c0
to
d0e10cf
Compare
The tests pass.. I'm roaming so there's some latency / timeout issues - I'll re-run them when I'm on a stable connection..
One thing of note - if you create an Event Hub Namespace, and subsequently delete it - you can't create it (in a new subscription, at least) for 7 days - as noted in this screenshot. Is it worth mentioning this in the docs? |
👍 all good - not sure why Travis is failing |
bd4dbee
to
fbbb030
Compare
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.
This LGTM! I have 1 suggested addition - a test for import :)
When we add that (to prove the import works!) we can get it merged
P.
Read: resourceArmEventHubNamespaceRead, | ||
Update: resourceArmEventHubNamespaceCreate, | ||
Delete: resourceArmEventHubNamespaceDelete, | ||
Importer: &schema.ResourceImporter{ |
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.
Can you add a test to make sure that the import works?
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.
Added a test but it's failing - posting this here for later as a reminder of what needs fixing:
TF_ACC=1 go test ./builtin/providers/azurerm -v -run TestAccAzureRMEventHubNamespace_importBasic -timeout 120m
=== RUN TestAccAzureRMEventHubNamespace_importBasic
--- FAIL: TestAccAzureRMEventHubNamespace_importBasic (371.43s)
testing.go:265: Step 1 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.
(map[string]string) {
}
(map[string]string) (len=2) {
(string) (len=8) "location": (string) (len=6) "westus",
(string) (len=19) "resource_group_name": (string) (len=28) "acctestRG-342930116917413830"
}
FAIL
exit status 1
FAIL github.com/hashicorp/terraform/builtin/providers/azurerm 371.449s
"sku": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
ForceNew: false, |
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.
ForceNew is false by default :)
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.
Fixed this.
Returning the string value, rather than the Azure SDK internal value
472d38c
to
b0e9c78
Compare
Parsing the location out to the friendly name
This LGTM now :) Thanks for the extra work here! I am going to rebase this PR to a single commit to keep the commit log clean :)
P. |
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. |
Added support for Event Hubs Namespaces:
I started / was hoping to add support for Event Hubs / Consumer Groups / Authorization Rules in the same PR - but there's bugs in the Azure SDK for Go meaning this isn't possible right now.. so I'll send a follow up when it's fixed :)