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

Allow 0 capacity for servicebus #2920

Merged
merged 5 commits into from
Mar 7, 2019

Conversation

hbuckle
Copy link
Contributor

@hbuckle hbuckle commented Feb 20, 2019

Allow the capacity to be set to 0 for service bus namespace. This is so we can create a module with the option for the premium sku - currently we have to maintain 2 modules.

Example module

variable "sku" {
  default = "Basic"
}

variable "capacity" {
  default = 0
}

resource "azurerm_servicebus_namespace" "test" {
  name                = "name"
  location            = "${azurerm_resource_group.test.location}"
  resource_group_name = "${azurerm_resource_group.test.name}"
  sku                 = "${var.sku}"
  capacity            = "${var.capacity}"
}

Usage

module "servicebusbasic" {
  sku = "Basic"
}

module "servicebuspremium" {
  sku      = "Premium"
  capacity = 4
}

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 @hbuckle,

Thank you for this PR, however as i understand it 0 is not a valid value for capacity?

Would it not be possible to default to 1 (the basic sku capacity) instead of 0?

@hbuckle
Copy link
Contributor Author

hbuckle commented Feb 21, 2019

@katbyte technically you can only set capacity for the premium sku and it has to be 1,2,4
The API will accept 0 for basic or standard but it has no effect - so this is kind of a workaround but it enables the scenario outlined above to have a single module that supports multiple skus

@katbyte
Copy link
Collaborator

katbyte commented Feb 21, 2019

@hbuckle,

Tried setting a basic SKU to 1 and see what your saying now, API just sets it to a 0. Could we put the validation back in and instead have it enforce 0 for basic and 1,2,4 for the premium SKUs? As well as adding a note to the documentation stating this too.

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 👍

@tombuildsstuff
Copy link
Contributor

@katbyte

Could we put the validation back in and instead have it enforce 0 for basic and 1,2,4 for the premium SKUs? As well as adding a note to the documentation stating this too.

Taking a look through I believe this is now covered, since the validation method only allows the 4 possible values (0, 1, 2 and 4) and the Create method validates it's either 0 or not - as such I think this is fine? 👍

@hbuckle
Copy link
Contributor Author

hbuckle commented Feb 22, 2019

Correct, the validation has moved to create and should only allow 0 for non-premium skus

@tombuildsstuff tombuildsstuff modified the milestones: 1.23.0, 1.25.0 Mar 5, 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.

hey @hbuckle

Thanks for pushing those changes - this now LGTM 👍

Thanks!

@tombuildsstuff tombuildsstuff modified the milestones: 1.25.0, 1.23.0 Mar 7, 2019
@tombuildsstuff
Copy link
Contributor

Tests pass:

screenshot 2019-03-07 at 11 18 34

@tombuildsstuff tombuildsstuff merged commit 638d890 into hashicorp:master Mar 7, 2019
tombuildsstuff added a commit that referenced this pull request Mar 7, 2019
@ghost
Copy link

ghost commented Mar 8, 2019

This has been released in version 1.23.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.23.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Apr 6, 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 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 6, 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.

3 participants