New Resource: SignalR service azurerm_signalr_service

[JunyiYi]

In this pull request, I introduced Azure SignalR (used to create real-time web application) to Terraform. AFAIK, there are no issues or feature requests yet.

I put it under "Messaging" section because it is somehow similar to notification hub.

[katbyte]

Hi @JunyiYi,

Thanks for the new resource, i've taken a quick look and left some comments inline.

[tombuildsstuff]

hey @JunyiYi

Thanks for this PR :)

I've taken a look through and left some comments in-line - but most of these comments are fairly minor; if we can fix those up & split the SKU block out we should be able to run the tests 👍

Thanks!

[JunyiYi]

Thanks @tombuildsstuff and @katbyte for reviewing the code. I have updated it accordingly, please take a look.

[WodansSon]

@JunyiYi Thanks for the PR LGTM!

[katbyte]

Is there a reason we don't just have the user use Free_F1 for the name property?

Also is it Free_F1 for all capacities?

[JunyiYi]

It is what @tombuildsstuff suggested before:

sku {
  name = "Free"
  capacity = 1
}

And for your second question, the answer is "Yes". SignalR service does not follow the convention of sku, they only accept Free_F1 and Standard_S1 no matter what capacity is.

[JunyiYi]

@tombuildsstuff @katbyte , after talking with the service team, they do need to use fixed sku names (i.e. Free_F1 and Standard_S1) for all capacities, because they want to adapt capacity changes easily. Quote from service team:

Initially capacity is allowed to be 1-10, now it’s currency values (1,2,5,...,100). We may add more supported numbers in future). To keep the same SKU name, we can support more (or less) units by slightly updating our UI/Validation logic while keeping the UX/interface and consistent and stable.

So I will change the sku name to only allow Free_F1 and Standard_S1.

[katbyte]

Sounds good @JunyiYi

[JunyiYi]

@tombuildsstuff @katbyte Code change has been pushed. One more thing, what service labeled should be added to this PR? If no existing label is applicable, could you please add a new service/signalr label?

[metacpp]

LGTM.

[katbyte]

@JunyiYi,

Looks good aside from two comments i've left inline

[katbyte]

@JunyiYi,

Looks good aside from two comments i've left inline

[katbyte]

Thanks for the changes @JunyiYi, LGTM 👍

[katbyte]

Tests pass:

[tombuildsstuff]

if input's nil we're going to set an empty sku block in the state (so there'll be an object there but it'll be empty) - can we update this to:

if input == nil {
  return []interface{}{}
}

...

this way it'll show that the Sku block isn't being returned from the API from the fact that Terraform will show the block being removed, rather than the fields being blank

[ghost]

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!