New Resource: azurerm_bot_channel_slack
#4367
Conversation
mbfrahry
commented
Sep 18, 2019
•
mbfrahry
commented
There was a problem hiding this comment.
hey @mbfrahry
Taking a look through this mostly LGTM - if we can fix up the comments this should otherwise be good to merge 👍
Thanks!
| @@ -195,6 +195,7 @@ func Provider() terraform.ResourceProvider { | |||
| "azurerm_batch_account": resourceArmBatchAccount(), | |||
| "azurerm_batch_application": resourceArmBatchApplication(), | |||
| "azurerm_batch_certificate": resourceArmBatchCertificate(), | |||
| "azurerm_bot_channel_slack": resourceArmBotChannelSlack(), | |||
| "azurerm_bot_channels_registration": resourceArmBotChannelsRegistration(), | |||
There was a problem hiding this comment.
should these both be channel_ or channels_?
There was a problem hiding this comment.
I don't think so. channels_registration houses every channel
| }, | ||
| ChannelName: botservice.ChannelNameSlackChannel1, | ||
| }, | ||
| Location: utils.String(d.Get("location").(string)), |
There was a problem hiding this comment.
we need to normalize this via:
| Location: utils.String(d.Get("location").(string)), | |
| Location: utils.String(azure.NormalizeLocation(d.Get("location").(string))), |
| return fmt.Errorf("Error issuing create request for Bot Channel Slack (Resource Group %q / Bot %q): %+v", resourceGroup, botName, err) | ||
| } | ||
|
|
||
| resp, err := client.Get(ctx, resourceGroup, botName, string(botservice.ChannelNameSlackChannel)) |
There was a problem hiding this comment.
do we need to cast this to a string for the Create too?
There was a problem hiding this comment.
No, the SDK is weird in that create/update takes the ChannelName constant while the get/delete take strings instead. It's weird
| return nil | ||
| } | ||
|
|
||
| return fmt.Errorf("Error reading Bot Channel Slack (Resource Group %q / Bot %q): %+v", id.ResourceGroup, botName, err) |
There was a problem hiding this comment.
can we match the ordering with other resources:
| return fmt.Errorf("Error reading Bot Channel Slack (Resource Group %q / Bot %q): %+v", id.ResourceGroup, botName, err) | |
| return fmt.Errorf("Error reading Channel Slack for Bot %q (Resource Group %q): %+v", botName, id.ResourceGroup, err) |
|
|
||
| if props := resp.Properties; props != nil { | ||
| if channel, ok := props.AsSlackChannel(); ok { | ||
| if channel != nil { |
There was a problem hiding this comment.
I think we can remove this line, since otherwise ok would return false?
| return fmt.Errorf("Error issuing create request for Bot Channel Slack (Resource Group %q / Bot %q): %+v", resourceGroup, botName, err) | ||
| } | ||
|
|
||
| resp, err := client.Get(ctx, resourceGroup, botName, string(botservice.ChannelNameSlackChannel)) |
There was a problem hiding this comment.
(same here) do we need to cast this to a string in all the calls? it's odd it's inconsistent
There was a problem hiding this comment.
Only because the SDK is inconsistent
| page_title: "Azure Resource Manager: azurerm_bot_channel_slack" | ||
| sidebar_current: "docs-azurerm-resource-bot-channel-slack" | ||
| description: |- | ||
| Manages a Bot Channel for Slack. |
There was a problem hiding this comment.
does this want to be:
| Manages a Bot Channel for Slack. | |
| Manages a Slack integration for a Bot Channel |
|
|
||
| # azurerm_bot_connection | ||
|
|
||
| Manages a Bot Channel for Slack. |
|
|
||
| ## Import | ||
|
|
||
| Bot Channel can be imported using the `resource id`, e.g. |
There was a problem hiding this comment.
does this want to be:
| Bot Channel can be imported using the `resource id`, e.g. | |
| The Slack Channel for a Bot can be imported using the `resource id`, e.g. |
|
This has been released in version 1.35.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.35.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 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
