-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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 resources: azurerm_data_lake_analytics_account & azurerm_data_lake_analytics_firewall_rule #1618
Conversation
…e_analytics_firewall_rule
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.
Some minor comments, but otherwise LGTM
|
||
import ( | ||
"fmt" | ||
"github.com/Azure/azure-sdk-for-go/services/datalake/analytics/mgmt/2016-11-01/account" |
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.
these imports probably need some minor reordering.
client := meta.(*ArmClient).dataLakeAnalyticsAccountClient | ||
ctx := meta.(*ArmClient).StopContext | ||
|
||
log.Printf("[INFO] preparing arguments for Azure ARM Date Lake Store creation.") |
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.
maybe add the name to this log?
client := meta.(*ArmClient).dataLakeAnalyticsFirewallRulesClient | ||
ctx := meta.(*ArmClient).StopContext | ||
|
||
log.Printf("[INFO] preparing arguments for Date Lake Analytics Firewall Rule creation.") |
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 name to this log?
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/response" | ||
"github.com/terraform-providers/terraform-provider-azurerm/azurerm/utils" | ||
"log" |
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.
weird ordering here
Not sure why Travis failed, hmmm... |
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.
Noticed a couple of tests looked invalid when scrolling through prior to running the tests for this, but it's otherwise 👍
|
||
tier = "Commitment100AUHours" | ||
|
||
default_store_account_name = "${azurerm_data_lake_store |
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 test is invalid?
resource_group_name = "${azurerm_resource_group.test.name}" | ||
location = "${azurerm_resource_group.test.location}" | ||
|
||
default_store_account_name = "${azurerm_data_lake_store |
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.
(same here)
resource_group_name = "${azurerm_resource_group.test.name}" | ||
location = "${azurerm_resource_group.test.location}" | ||
|
||
default_store_account_name = "${azurerm_data_lake_store |
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.
(same here)
2d61d0d
to
d05a178
Compare
tests pass:
|
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! |
No description provided.