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

provider/azurerm: Setting an optional field on Service Bus Topic #13223

Merged

Conversation

tombuildsstuff
Copy link
Contributor

@tombuildsstuff tombuildsstuff commented Mar 30, 2017

The field duplicate_detection_history_time_window is defaulted at the API to 00:10:00

Due a bug in the current version of the Azure SDK, this value wasn't being surfaced correctly, however we're going to need to make this optional by default

(also fixing the logging to output the error's)

@tombuildsstuff tombuildsstuff force-pushed the azurerm-servicebustopic branch from 90c8f8c to 2a3f162 Compare March 31, 2017 19:25
@tombuildsstuff tombuildsstuff changed the title [WIP] provider/azurerm: Setting an optional field on Service Bus Topic provider/azurerm: Setting an optional field on Service Bus Topic Mar 31, 2017
@tombuildsstuff tombuildsstuff requested a review from stack72 March 31, 2017 20:13
@tombuildsstuff
Copy link
Contributor Author

tombuildsstuff commented Mar 31, 2017

Tests pass:

$ envchain azurerm make testacc TEST=./builtin/providers/azurerm TESTARGS='-run=TestAccAzureRMServiceBusTopic'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/03/31 20:25:40 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/azurerm -v -run=TestAccAzureRMServiceBusTopic -timeout 120m
=== RUN   TestAccAzureRMServiceBusTopic_importBasic
--- PASS: TestAccAzureRMServiceBusTopic_importBasic (339.46s)
=== RUN   TestAccAzureRMServiceBusTopic_basic
--- PASS: TestAccAzureRMServiceBusTopic_basic (320.62s)
=== RUN   TestAccAzureRMServiceBusTopic_update
--- PASS: TestAccAzureRMServiceBusTopic_update (349.12s)
=== RUN   TestAccAzureRMServiceBusTopic_enablePartitioningStandard
--- PASS: TestAccAzureRMServiceBusTopic_enablePartitioningStandard (476.61s)
=== RUN   TestAccAzureRMServiceBusTopic_enablePartitioningPremium
--- PASS: TestAccAzureRMServiceBusTopic_enablePartitioningPremium (643.32s)
=== RUN   TestAccAzureRMServiceBusTopic_enableDuplicateDetection
--- PASS: TestAccAzureRMServiceBusTopic_enableDuplicateDetection (340.50s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/azurerm	2469.647s

@tombuildsstuff tombuildsstuff changed the title provider/azurerm: Setting an optional field on Service Bus Topic [WIP] provider/azurerm: Setting an optional field on Service Bus Topic Mar 31, 2017
@tombuildsstuff tombuildsstuff removed the request for review from stack72 March 31, 2017 20:13
@tombuildsstuff tombuildsstuff changed the title [WIP] provider/azurerm: Setting an optional field on Service Bus Topic provider/azurerm: Setting an optional field on Service Bus Topic Mar 31, 2017
@tombuildsstuff tombuildsstuff requested a review from stack72 March 31, 2017 20:55
@@ -55,6 +55,7 @@ func resourceArmServiceBusTopic() *schema.Resource {
"duplicate_detection_history_time_window": {
Type: schema.TypeString,
Optional: true,
Computed: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

So we are not setting the default value? We are just getting it from Azure API?

Copy link
Contributor Author

@tombuildsstuff tombuildsstuff Apr 10, 2017

Choose a reason for hiding this comment

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

Yeah - unfortunately setting this to Optional: true, Default: "00:10:00" leaves a diff in the Import test:

TF_ACC=1 go test ./builtin/providers/azurerm/ -v -run=TestAccAzureRMServiceBusTopic_import -timeout 120m
=== RUN   TestAccAzureRMServiceBusTopic_importBasic
--- FAIL: TestAccAzureRMServiceBusTopic_importBasic (349.48s)
	testing.go:280: Step 1 error: ImportStateVerify attributes not equivalent. Difference is shown below. Top is actual, bottom is expected.

		(map[string]string) {
		}


		(map[string]string) (len=1) {
		 (string) (len=39) "duplicate_detection_history_time_window": (string) (len=8) "00:10:00"
		}

FAIL
exit status 1
FAIL	github.com/hashicorp/terraform/builtin/providers/azurerm	349.497s

@stack72
Copy link
Contributor

stack72 commented Apr 10, 2017

LGTM! :shipit:

@tombuildsstuff tombuildsstuff merged commit 906cd09 into hashicorp:master Apr 10, 2017
@tombuildsstuff tombuildsstuff deleted the azurerm-servicebustopic branch April 10, 2017 13:35
tombuildsstuff added a commit that referenced this pull request Apr 10, 2017
@ghost
Copy link

ghost commented Apr 14, 2020

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

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

2 participants