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

new resource: azurerm_kusto_iothub_data_connection #8626

Merged
merged 3 commits into from
Feb 22, 2021

Conversation

njuCZ
Copy link
Contributor

@njuCZ njuCZ commented Sep 25, 2020

refactor kusto RP and add new resource
image

@ghost ghost added the size/XXL label Sep 25, 2020
@njuCZ njuCZ marked this pull request as draft September 25, 2020 07:44
@njuCZ njuCZ marked this pull request as ready for review September 25, 2020 08:10
@njuCZ njuCZ force-pushed the kusto_iothub_data_connection branch from fc139c2 to 27db5e7 Compare September 25, 2020 08:20
@njuCZ njuCZ force-pushed the kusto_iothub_data_connection branch from 27db5e7 to 8a5e92c Compare September 25, 2020 08:24
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @njuCZ - overall this looks good but could we get a complete and update test? thanks!

"github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/acceptance"
)

func TestAccAzureRMKustoIotHubDataConnection_basic(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add a complete test which sets all optional properties and then an update test that goes basic -> complete -> basic?

@njuCZ
Copy link
Contributor Author

njuCZ commented Oct 9, 2020

@katbyte sorry for late reply. complete test is hard for this resource, because the optional fields "table_name", "mapping_rule_name" are kusto RP data plane resource. Fow now, we could not create them through terraform. and I found existing resource azurerm_kusto_eventhub_data_connection keeps these fields and no complete acctest.

Do you think we would better delete these fields or just keep them and no test?

@ghost ghost removed the waiting-response label Oct 9, 2020
@jackofallops jackofallops modified the milestones: v2.32.0, v2.33.0 Oct 15, 2020
@tombuildsstuff tombuildsstuff removed this from the v2.33.0 milestone Oct 21, 2020
@njuCZ njuCZ requested a review from katbyte October 27, 2020 06:00
@katbyte
Copy link
Collaborator

katbyte commented Oct 27, 2020

@njuCZ - can we just delete the properties we cannot test and leave the rest for now? thanks

@njuCZ njuCZ force-pushed the kusto_iothub_data_connection branch from 801c092 to 14de6d4 Compare October 28, 2020 03:42
@njuCZ
Copy link
Contributor Author

njuCZ commented Oct 28, 2020

@katbyte Thanks for your advice, I have deleted these fields. Please have a look again when free.

@njuCZ njuCZ force-pushed the kusto_iothub_data_connection branch from 14de6d4 to 49fcee0 Compare December 16, 2020 04:21
@njuCZ
Copy link
Contributor Author

njuCZ commented Dec 16, 2020

just rebased the latest master branch and resolve conflicts

@katbyte
Copy link
Collaborator

katbyte commented Feb 19, 2021

@njuCZ - this is failing to build with:

[15:38:08]	[Step 4/5] ./kusto_cluster_customer_managed_key_resource.go:11:2: imported and not used: "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/azure"
[15:38:08]	[Step 4/5] ./kusto_eventhub_data_connection_resource.go:16:2: validate redeclared as imported package name
[15:38:08]	[Step 4/5] 	previous declaration at ./kusto_eventhub_data_connection_resource.go:14:2
[15:38:08]	[Step 4/5] ./kusto_eventhub_data_connection_resource.go:90:19: undefined: "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/kusto/validate".ValidateEventHubConsumerName
[15:39:30]	[Step 4/5] Process exited with code 2
[15:39:30]	[Step 4/5] Process exited with code 2 (Step: Compile Test Binary (Command Line))
[15:39:30]	[Step 4/5] Step Compile Test Binary (Command Line) failed
[15:39:30]	[Step 4/5] Error message is logged

@njuCZ njuCZ force-pushed the kusto_iothub_data_connection branch from 49fcee0 to cb1710e Compare February 20, 2021 02:47
@njuCZ
Copy link
Contributor Author

njuCZ commented Feb 20, 2021

@katbyte I have rebased this PR, now it could build successfully and acctest could pass.
image

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @njuCZ - LGTM 👍

@katbyte katbyte added this to the v2.49.0 milestone Feb 22, 2021
@katbyte katbyte merged commit 557487f into hashicorp:master Feb 22, 2021
katbyte added a commit that referenced this pull request Feb 22, 2021
@ghost
Copy link

ghost commented Feb 26, 2021

This has been released in version 2.49.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.49.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Mar 25, 2021

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 ghost locked as resolved and limited conversation to collaborators Mar 25, 2021
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.

None yet

4 participants