service_bus_queue: support for max_delivery_count
#2028
Conversation
There was a problem hiding this comment.
hey @tkbky
Thanks for this PR - I've taken a look through and this mostly LGTM - if we can fix up the test so the configuration is valid we can kick off the tests and get this merged :)
Thanks!
| name = "acctestservicebusqueue-%d" | ||
| resource_group_name = "${azurerm_resource_group.test.name}" | ||
| namespace_name = "${azurerm_servicebus_namespace.test.name}" | ||
| maxDeliveryCount = 20 |
There was a problem hiding this comment.
can we update this to max_delivery_count to match the schema field?
| "max_delivery_count": { | ||
| Type: schema.TypeInt, | ||
| Optional: true, | ||
| Default: 10, |
There was a problem hiding this comment.
out of interest where does this default value come from? (and if so, are there any validation requirements for this field?)
There was a problem hiding this comment.
It's coming from the doc over here, not sure about the validation requirements though.
There was a problem hiding this comment.
I was able to set it to 999999999 but it does have to be at least 1 so we could go with validation.IntAtleast(1)?
There was a problem hiding this comment.
@katbyte yes, having it to be at least 1 makes sense to me.
|
@tombuildsstuff Thanks for reviewing the PR. I've made the requested changes. |
|
@katbyte It's my miss. Thanks for the catch. I've updated the PR to include the import check step, and include the max delivery count when read. |
| func testAccAzureRMServiceBusQueue_maxDeliveryCount(rInt int, location string) string { | ||
| return fmt.Sprintf(` | ||
| resource "azurerm_resource_group" "test" { | ||
| name = "acctestRG-%d" |
There was a problem hiding this comment.
Could we align all the assignments in the TF code to match the other tests?
There was a problem hiding this comment.
Definitely. Sorry for not noticing the inconsistency.
tombuildsstuff
dismissed
katbyte’s stale review
dismissing since changes have been pushed
|
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 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked and limited conversation to collaborators
This change adds
max_delivery_countto the service bus queue's schema so that it is configurable.See #1977