Event hub namespace with KafkaEnabled property #2395

fedeoliv · 2018-11-27T17:16:32Z

Implemented feature request #1417 with kafkaEnabled property

(fixes #1417)

katbyte

Hi @fernandoBRS,

Thank you for adding this property! In addition to a few minor comments left inline could we add he new property to the documentation?

It also looks like you are going to need to vender in the newer API version supporting the kafka_enabled property

azurerm/data_source_eventhub_namespace_test.go

azurerm/resource_arm_eventhub_namespace_test.go

Co-Authored-By: fernandoBRS <fedeoliv@microsoft.com>

fedeoliv · 2018-11-27T18:33:12Z

Hi @katbyte, yes I can add it in the documentation. Is it better to do a different PR for the documentation?

katbyte · 2018-11-27T21:42:25Z

Same PR is fine @fernandoBRS 🙂

fedeoliv · 2018-11-28T15:52:52Z

@katbyte the preview version of the Event Hub that supports Kafka has a model structure a bit different compared to the current supported version on Terraform. For example, we can compare the models.go from the 2017-04-01 version with the models.go from the preview version.

I would suggest two approaches:

Create a azurerm_eventhub_preview_namespace
Refactor the azurerm_eventhub_namespace to use the new API

Do you have any guidance in these approaches?

Thanks!

katbyte · 2018-11-28T21:34:45Z

@fernandoBRS,

Looking at the two files the only difference seems to be the additional two fields:

I'm not sure what refactoring would be required aside from adding property?

katbyte · 2018-12-07T04:19:53Z

the 2017-04-01/eventhub package gets this property in version 22 of the sdk. It might be best to wait until we upgrade.

jackie-goldschmidt

Should there be tests?

jackie-goldschmidt · 2018-12-07T21:18:07Z

azurerm/data_source_eventhub_namespace_test.go

@@ -83,7 +84,9 @@ resource "azurerm_eventhub_namespace" "test" {
  resource_group_name      = "${azurerm_resource_group.test.name}"
  sku                      = "Standard"
  capacity                 = "2"
-  auto_inflate_enabled     = true
+	auto_inflate_enabled     = true


Looks like line 86 and 87 are duplicates.

A change like this usually means the line break when from \n <--> \n\r

Sorry, I don't understand. Are you saying lines 87 and 88 are not duplicates, but appear so due to github formatting? It looks to me like they are two green + lines with the same content.

Oh my apologies, you are definitely correct. Line 88 didn't appear in the conversation view.

azurerm/resource_arm_eventhub_namespace_test.go

katbyte · 2018-12-07T21:58:24Z

Yep, this should definitely be added to a test

jackie-goldschmidt

Please provide tests that prove this change works as expected.

jackie-goldschmidt

Please provide tests that prove this change works as expected.

jackie-goldschmidt · 2018-12-10T15:02:21Z

azurerm/data_source_eventhub_namespace_test.go

@@ -83,7 +84,9 @@ resource "azurerm_eventhub_namespace" "test" {
  resource_group_name      = "${azurerm_resource_group.test.name}"
  sku                      = "Standard"
  capacity                 = "2"
-  auto_inflate_enabled     = true
+	auto_inflate_enabled     = true


Sorry, I don't understand. Are you saying lines 87 and 88 are not duplicates, but appear so due to github formatting? It looks to me like they are two green + lines with the same content.

katbyte · 2018-12-28T03:25:37Z

Depends on #2572

katbyte

@fernandoBRS,

We have updated the SDK to v24 and unblocked this. I hope you don't mind but i made the required changes 🙂 LGTM now 🚀

fedeoliv · 2019-01-09T09:41:08Z

Good news, thanks for the support @katbyte ! 🚀

ghost · 2019-03-05T14:10:56Z

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!

Event hub namespace with KafkaEnabled propperty

5a1782d

ghost added the size/M label Nov 27, 2018

fedeoliv mentioned this pull request Nov 27, 2018

Feature Request : Azure Event Hub kafka Enabled #1417

Closed

fedeoliv changed the title ~~Event hub namespace with KafkaEnabled propperty~~ Event hub namespace with KafkaEnabled property Nov 27, 2018

katbyte requested changes Nov 27, 2018

View reviewed changes

azurerm/data_source_eventhub_namespace_test.go Outdated Show resolved Hide resolved

azurerm/resource_arm_eventhub_namespace_test.go Show resolved Hide resolved

azurerm/resource_arm_eventhub_namespace_test.go Outdated Show resolved Hide resolved

katbyte added this to the 1.20.0 milestone Nov 27, 2018

katbyte added enhancement waiting-response service/event-hubs EventHubs labels Nov 27, 2018

katbyte and others added 3 commits November 27, 2018 12:30

Update azurerm/resource_arm_eventhub_namespace_test.go

1c03b97

Co-Authored-By: fernandoBRS <fedeoliv@microsoft.com>

Update azurerm/data_source_eventhub_namespace_test.go

87154e5

Co-Authored-By: fernandoBRS <fedeoliv@microsoft.com>

Update azurerm/resource_arm_eventhub_namespace_test.go

4e92e9f

Co-Authored-By: fernandoBRS <fedeoliv@microsoft.com>

ghost removed the waiting-response label Nov 27, 2018

katbyte added the waiting-response label Nov 27, 2018

ghost removed the waiting-response label Nov 28, 2018

katbyte added the waiting-response label Nov 28, 2018

tombuildsstuff modified the milestones: 1.20.0, 1.21.0 Dec 6, 2018

katbyte modified the milestones: 1.21.0, Blocked Dec 7, 2018

katbyte self-assigned this Dec 7, 2018

jackie-goldschmidt reviewed Dec 7, 2018

View reviewed changes

jackie-goldschmidt suggested changes Dec 10, 2018

View reviewed changes

katbyte added upstream/microsoft Indicates that there's an upstream issue blocking this issue/PR and removed upstream/microsoft Indicates that there's an upstream issue blocking this issue/PR labels Jan 6, 2019

katbyte modified the milestones: Blocked, 1.21.0 Jan 8, 2019

katbyte added 3 commits January 8, 2019 23:01

merge master -S

6073ff2

Merge branch 'master' into fernandobrs/eventhub-kafkaenabled

f06ca18

added kafka enabled property to docs

ca40b7b

ghost added the documentation label Jan 9, 2019

katbyte approved these changes Jan 9, 2019

View reviewed changes

katbyte merged commit 3012599 into hashicorp:master Jan 9, 2019

katbyte added a commit that referenced this pull request Jan 9, 2019

Update CHANGELOG.md to include #2395

0c3b045

tombuildsstuff removed the waiting-response label Jan 9, 2019

fedeoliv mentioned this pull request Jan 9, 2019

Azure Event Hub documentation: Kafka enabled property added on namespace #2627

Merged

ghost locked and limited conversation to collaborators Mar 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Event hub namespace with KafkaEnabled property #2395

Event hub namespace with KafkaEnabled property #2395

fedeoliv commented Nov 27, 2018 •

edited by katbyte

Loading

katbyte left a comment •

edited

Loading

fedeoliv commented Nov 27, 2018

katbyte commented Nov 27, 2018

fedeoliv commented Nov 28, 2018

katbyte commented Nov 28, 2018

katbyte commented Dec 7, 2018

jackie-goldschmidt left a comment

jackie-goldschmidt Dec 7, 2018

katbyte Dec 7, 2018

jackie-goldschmidt Dec 10, 2018

katbyte Dec 10, 2018

katbyte commented Dec 7, 2018

jackie-goldschmidt left a comment

jackie-goldschmidt left a comment

jackie-goldschmidt Dec 10, 2018

katbyte commented Dec 28, 2018

katbyte left a comment

fedeoliv commented Jan 9, 2019

ghost commented Mar 5, 2019

Event hub namespace with KafkaEnabled property #2395

Event hub namespace with KafkaEnabled property #2395

Conversation

fedeoliv commented Nov 27, 2018 • edited by katbyte Loading

katbyte left a comment • edited Loading

Choose a reason for hiding this comment

fedeoliv commented Nov 27, 2018

katbyte commented Nov 27, 2018

fedeoliv commented Nov 28, 2018

katbyte commented Nov 28, 2018

katbyte commented Dec 7, 2018

jackie-goldschmidt left a comment

Choose a reason for hiding this comment

jackie-goldschmidt Dec 7, 2018

Choose a reason for hiding this comment

katbyte Dec 7, 2018

Choose a reason for hiding this comment

jackie-goldschmidt Dec 10, 2018

Choose a reason for hiding this comment

katbyte Dec 10, 2018

Choose a reason for hiding this comment

katbyte commented Dec 7, 2018

jackie-goldschmidt left a comment

Choose a reason for hiding this comment

jackie-goldschmidt left a comment

Choose a reason for hiding this comment

jackie-goldschmidt Dec 10, 2018

Choose a reason for hiding this comment

katbyte commented Dec 28, 2018

katbyte left a comment

Choose a reason for hiding this comment

fedeoliv commented Jan 9, 2019

ghost commented Mar 5, 2019

fedeoliv commented Nov 27, 2018 •

edited by katbyte

Loading

katbyte left a comment •

edited

Loading