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

Added Speech Services Kind #2583

Merged
merged 2 commits into from
Jan 10, 2019
Merged

Added Speech Services Kind #2583

merged 2 commits into from
Jan 10, 2019

Conversation

lqdev
Copy link
Contributor

@lqdev lqdev commented Dec 30, 2018

When trying to provision a resource of Speech kind, there are errors. To correct for this, I added kind SpeechServices which should be compatible with the kind being used by Azure currently for that resource.

When trying to provision a resource of `Speech` kind, there are errors. To correct for this, I added kind `SpeechServices` which should be compatible with the kind being used by Azure currently for that resource.
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.

Hi @lqdev,

Thank you for this addition. Looking at the resource & SDK its clear it needs to be updated to use the SDK enum values as I don't see Speech, but then I also don't see SpeechServices. Is this a new addition to the service? because the possible values the SDK recognizes are:

	// BingSpeech ...
	BingSpeech Kind = "Bing.Speech"
	// CustomSpeech ...
	CustomSpeech Kind = "CustomSpeech"
	// SpeakerRecognition ...
	SpeakerRecognition Kind = "SpeakerRecognition"
	// SpeechTranslation ...
	SpeechTranslation Kind = "SpeechTranslation"

The documentation will require updating with any new values.

@neilbailie
Copy link

Hi,

I've hit this problem before and raised an issue see -> #2294 which covers the origin of the missing kind.

Neil

@ghost ghost removed the waiting-response label Dec 31, 2018
@lqdev
Copy link
Contributor Author

lqdev commented Dec 31, 2018

Thanks @katbyte and @neilbailie for providing more clarification. At the time of this addition I looked at the output form Azure CLI for the kind of service I wanted to deploy. I was under the impression that this issue was only on the terraform end but was not aware it also extended to the Azure API. I'll sit tight and wait for the changes to be implemented 😃

@katbyte katbyte added the upstream/microsoft Indicates that there's an upstream issue blocking this issue/PR label Dec 31, 2018
@katbyte katbyte added this to the Blocked milestone Dec 31, 2018
@katbyte
Copy link
Collaborator

katbyte commented Dec 31, 2018

@lqdev,

This will be a quick fix once #2572 is merged, however that is currently blocked by a weird API versioning issue so marking this as such.

@katbyte katbyte removed this from the Blocked milestone Jan 9, 2019
@katbyte katbyte removed the upstream/microsoft Indicates that there's an upstream issue blocking this issue/PR label Jan 9, 2019
@katbyte
Copy link
Collaborator

katbyte commented Jan 9, 2019

@lqdev,

We have updated the SDK so this should be unblocked now 🙂

@lqdev
Copy link
Contributor Author

lqdev commented Jan 10, 2019

Thanks! @katbyte

```
$ acctests azurerm TestAccAzureRMCognitiveAccount_speechServices
=== RUN   TestAccAzureRMCognitiveAccount_speechServices
=== PAUSE TestAccAzureRMCognitiveAccount_speechServices
=== CONT  TestAccAzureRMCognitiveAccount_speechServices
--- PASS: TestAccAzureRMCognitiveAccount_speechServices (83.54s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	84.821s
```
@ghost ghost added size/S documentation and removed size/XS labels Jan 10, 2019
Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for this @lqdev! I hope you don't mind but I've pushed a commit adding a test and docs for this new value

@tombuildsstuff tombuildsstuff dismissed katbyte’s stale review January 10, 2019 15:54

dismissing since changes have been pushed

@tombuildsstuff tombuildsstuff merged commit ec86972 into hashicorp:master Jan 10, 2019
tombuildsstuff added a commit that referenced this pull request Jan 10, 2019
@ghost
Copy link

ghost commented Mar 5, 2019

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 ghost locked and limited conversation to collaborators Mar 5, 2019
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.

4 participants