support alias connection strings for eventhub resources #6708

neil-yechenwei · 2020-04-30T05:34:30Z

katbyte

thanks for the PR. I was wondering if it would make more sense to. put alias on the end so the properties sort and read better? thanks!

katbyte · 2020-05-05T03:54:55Z

azurerm/helpers/azure/eventhub.go

@@ -100,7 +100,13 @@ func EventHubAuthorizationRuleSchemaFrom(s map[string]*schema.Schema) map[string
 			Default:  false,
 		},

-		"primary_key": {
+		"alias_primary_connection_string": {


could we rename this to

Suggested change

"alias_primary_connection_string": {

"primary_connection_string_alias": {

katbyte · 2020-05-05T03:55:07Z

azurerm/helpers/azure/eventhub.go

+			Sensitive: true,
+		},
+
+		"alias_secondary_connection_string": {


and

Suggested change

"alias_secondary_connection_string": {

"secondary_connection_string_alias": {

sebader · 2020-05-05T07:28:51Z

thanks for the PR. I was wondering if it would make more sense to. put alias on the end so the properties sort and read better? thanks!

While I understand your argument, the way @neil-yechenwei implemented it is following the official SDK's naming: https://github.com/Azure/azure-sdk-for-go/blob/master/services/eventhub/mgmt/2017-04-01/eventhub/models.go#L226

what is the general guidance for the TF provider? Does it have its own naming strategy or does it try to follow the underlying APIs?

neil-yechenwei · 2020-05-06T00:47:31Z

The goal is to make users better understand these properties in TF.

neil-yechenwei · 2020-05-06T01:21:43Z

@katbyte , thanks for your comments. I've updated code.

katbyte · 2020-05-06T21:36:33Z

@sebader - we try to give properties the most clear accurate name we can, much like the portal often deviates from the SDK/APi we do as well. They are often poorly (and sometimes incorrectly!) named, and in this case putting it at the end allowed the propertied to group in documentation & schema provider a better UX all around.

katbyte

@neil-yechenwei, thanks for the changes. Looks like you missed some renames - ounce those have been updated this should be good to merge

katbyte · 2020-05-07T15:00:50Z

website/docs/r/eventhub_namespace.html.markdown

+* `alias_default_primary_connection_string` - The alias of the primary connection string for the authorization
+    rule `RootManageSharedAccessKey`, which is generated when disaster recovery is enabled.
+
+* `alias_default_secondary_connection_string` - The alias of the secondary connection string for the


I think you missed on here?

katbyte · 2020-05-07T15:00:53Z

website/docs/r/eventhub_namespace.html.markdown

@@ -88,14 +88,20 @@ The following attributes are exported:
 The following attributes are exported only if there is an authorization rule named
 `RootManageSharedAccessKey` which is created automatically by Azure.

+* `alias_default_primary_connection_string` - The alias of the primary connection string for the authorization


I think you missed on here?

katbyte · 2020-05-07T15:01:00Z

website/docs/d/eventhub_namespace.html.markdown

@@ -47,14 +47,20 @@ output "eventhub_namespace_id" {
 The following attributes are exported only if there is an authorization rule named
 `RootManageSharedAccessKey` which is created automatically by Azure.

-* `default_primary_connection_string` - The primary connection string for the authorization
+* `alias_default_primary_connection_string` - The alias of the primary connection string for the authorization


I think you missed on here?

katbyte · 2020-05-07T15:01:02Z

website/docs/d/eventhub_namespace.html.markdown

    rule `RootManageSharedAccessKey`.

-* `default_secondary_connection_string` - The secondary connection string for the
+* `alias_default_secondary_connection_string` - The alias of the secondary connection string for the


I think you missed on here?

katbyte · 2020-05-07T15:02:01Z

azurerm/internal/services/eventhub/eventhub_namespace_resource.go

@@ -158,13 +158,19 @@ func resourceArmEventHubNamespace() *schema.Resource {
 				},
 			},

-			"default_primary_connection_string": {
+			"alias_default_primary_connection_string": {


I think you missed on here?

katbyte · 2020-05-07T15:02:04Z

azurerm/internal/services/eventhub/eventhub_namespace_resource.go

 				Type:      schema.TypeString,
 				Computed:  true,
 				Sensitive: true,
 			},

-			"default_secondary_connection_string": {
+			"alias_default_secondary_connection_string": {


I think you missed on here?

katbyte · 2020-05-07T15:02:24Z

azurerm/internal/services/eventhub/eventhub_namespace_authorization_rule_data_source.go

-			"listen": {
-				Type:     schema.TypeBool,
-				Computed: true,
+			"primary_connection_string_alias": {


these are not ordered correctly?

katbyte · 2020-05-07T15:02:28Z

azurerm/helpers/azure/eventhub.go

@@ -100,7 +100,13 @@ func EventHubAuthorizationRuleSchemaFrom(s map[string]*schema.Schema) map[string
 			Default:  false,
 		},

-		"primary_key": {
+		"primary_connection_string_alias": {


these are not ordered correctly?

jackofallops · 2020-05-07T15:43:05Z

Running acctests now

katbyte

thanks for the fixes @jackofallops! LGTM 👍

ghost · 2020-05-08T01:41:21Z

This has been released in version 2.9.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.9.0"
}
# ... other configuration ...

sebader · 2020-05-08T07:38:51Z

@neil-yechenwei quick question on the usage of this new feature: When I do not know whether an EventHub is geo-replicated, is it safe to do it this way now? I.e. will the alias be null if not geo-replicated?

value = data.azurerm_eventhub_authorization_rule.example.primary_connection_string_alias != null ? data.azurerm_eventhub_authorization_rule.example.primary_connection_string_alias : data.azurerm_eventhub_authorization_rule.example.primary_connection_string

ghost · 2020-06-07T14:50:13Z

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 👉 [email protected]. Thanks!

neil-yechenwei added 4 commits April 26, 2020 10:03

Add support for alias connection string for eventhub

19b8c5f

Update code

6bffd2d

Update code

dfa9f4d

Update code

53f73ca

ghost added size/XL documentation labels Apr 30, 2020

katbyte requested changes May 5, 2020

View reviewed changes

katbyte added service/event-hubs EventHubs enhancement labels May 5, 2020

katbyte added this to the v2.9.0 milestone May 5, 2020

katbyte added the waiting-response label May 5, 2020

ghost removed the waiting-response label May 5, 2020

Update code

488aaa3

neil-yechenwei requested a review from katbyte May 6, 2020 01:21

git merge master

f5d64bf

katbyte requested changes May 7, 2020

View reviewed changes

katbyte added the waiting-response label May 7, 2020

review fixes

f984fe2

jackofallops removed the waiting-response label May 7, 2020

katbyte approved these changes May 7, 2020

View reviewed changes

katbyte changed the title ~~Add support for alias connection string for eventhub~~ support alias connection strings for eventhub resources May 7, 2020

katbyte merged commit 776a23d into hashicorp:master May 7, 2020

katbyte added a commit that referenced this pull request May 7, 2020

update CHANGELOG.md to include #6708

26b9791

ghost locked and limited conversation to collaborators Jun 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support alias connection strings for eventhub resources #6708

support alias connection strings for eventhub resources #6708

neil-yechenwei commented Apr 30, 2020

katbyte left a comment

katbyte May 5, 2020

neil-yechenwei May 6, 2020

katbyte May 5, 2020

neil-yechenwei May 6, 2020

sebader commented May 5, 2020 •

edited

Loading

neil-yechenwei commented May 6, 2020 •

edited

Loading

neil-yechenwei commented May 6, 2020

katbyte commented May 6, 2020

katbyte left a comment

katbyte May 7, 2020

katbyte May 7, 2020

katbyte May 7, 2020

katbyte May 7, 2020

katbyte May 7, 2020

katbyte May 7, 2020

katbyte May 7, 2020

katbyte May 7, 2020

jackofallops commented May 7, 2020

katbyte left a comment

ghost commented May 8, 2020

sebader commented May 8, 2020 •

edited

Loading

ghost commented Jun 7, 2020

	"alias_primary_connection_string": {
	"primary_connection_string_alias": {

	"alias_secondary_connection_string": {
	"secondary_connection_string_alias": {

support alias connection strings for eventhub resources #6708

support alias connection strings for eventhub resources #6708

Conversation

neil-yechenwei commented Apr 30, 2020

katbyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebader commented May 5, 2020 • edited Loading

neil-yechenwei commented May 6, 2020 • edited Loading

neil-yechenwei commented May 6, 2020

katbyte commented May 6, 2020

katbyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jackofallops commented May 7, 2020

katbyte left a comment

Choose a reason for hiding this comment

ghost commented May 8, 2020

sebader commented May 8, 2020 • edited Loading

ghost commented Jun 7, 2020

sebader commented May 5, 2020 •

edited

Loading

neil-yechenwei commented May 6, 2020 •

edited

Loading

sebader commented May 8, 2020 •

edited

Loading