New Resource: azurerm_data_factory
#3159
Conversation
ghost
added
There was a problem hiding this comment.
hey @mbfrahry
Thanks for this PR :)
Taking a look through this looks pretty good to me - if we can fix up the minor comments this should otherwise be good to merge 👍
Thanks!
|
Thanks for the review @tombuildsstuff! I believe I've addressed all your comments |
|
Any idea when this will make it into master? I'd like to take it to prod :-) |
tombuildsstuff
dismissed
their stale review
dismissing since changes have been pushed
|
Tests pass:
|
azurerm/resource_arm_data_factory.go
Outdated
| "tenant_id": { | ||
| Type: schema.TypeString, | ||
| Required: true, | ||
| ValidateFunc: validate.NoEmptyStrings, |
There was a problem hiding this comment.
Could this be validated as an ID?
azurerm/resource_arm_data_factory.go
Outdated
| return fmt.Errorf("Error setting `vsts_configuration`: %+v", err) | ||
| } | ||
| } else { | ||
| d.Set("vsts_configuration", []interface{}{}) |
There was a problem hiding this comment.
Should we move this and the
d.Set("github_configuration", []interface{}{})up above the two checks and then remove the else statements?
azurerm/resource_arm_data_factory.go
Outdated
| repositoryName := vsts["repository_name"].(string) | ||
| rootFolder := vsts["root_folder"].(string) | ||
| tenantID := vsts["tenant_id"].(string) | ||
| // https://github.com/Azure/go-autorest/issues/307 |
There was a problem hiding this comment.
minor expanding on this comment as to whats going on beyond a link could be useful
There was a problem hiding this comment.
Hbuckle found a weird case where you can pass the struct or the address of the struct into the sdk. If you didn't pass the address though, the sdk broke. I don't think we need to track that inside Terraform though since we can just pass the address like we normally do.
| resource "azurerm_data_factory" "test" { | ||
| name = "acctestdf%d" | ||
| location = "${azurerm_resource_group.test.location}" | ||
| resource_group_name = "${azurerm_resource_group.test.name}" |
There was a problem hiding this comment.
Could we fix the alignment here?
| resource "azurerm_data_factory" "test" { | ||
| name = "acctestdf%d" | ||
| location = "${azurerm_resource_group.test.location}" | ||
| resource_group_name = "${azurerm_resource_group.test.name}" |
|
Hey @archmangler, this will make it into the next release 1.25.0 |
|
This has been released in version 1.25.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 = "~> 1.25.0"
}
# ... other configuration ... |
|
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! |
Adding a new resource
azurerm_data_factoryThis is taking the majority of @hbuckle's work from #1720 and modifying it/updating tests.
Fixes #1720