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

azurerm_log_analytics_workspace_linked_service: fix casing issues #2594

Merged
merged 2 commits into from
Jan 5, 2019

Conversation

katbyte
Copy link
Collaborator

@katbyte katbyte commented Jan 4, 2019

fixes #2593

@katbyte
Copy link
Collaborator Author

katbyte commented Jan 4, 2019

tests:

[18:14:45] kt@snowbook:~/hashi/..3../terraform-providers/terraform-provider-azurerm▸log-analytics/uppercase$ testazure TestAccAzureRMLogAnalyticsWorkspaceLinkedService
==> Fixing source code with gofmt...
gofmt -s -w ./azurerm
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./azurerm -v -test.run=TestAccAzureRMLogAnalyticsWorkspaceLinkedService -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAzureRMLogAnalyticsWorkspaceLinkedService_basic
=== PAUSE TestAccAzureRMLogAnalyticsWorkspaceLinkedService_basic
=== RUN   TestAccAzureRMLogAnalyticsWorkspaceLinkedService_complete
=== PAUSE TestAccAzureRMLogAnalyticsWorkspaceLinkedService_complete
=== CONT  TestAccAzureRMLogAnalyticsWorkspaceLinkedService_basic
=== CONT  TestAccAzureRMLogAnalyticsWorkspaceLinkedService_complete
--- PASS: TestAccAzureRMLogAnalyticsWorkspaceLinkedService_basic (100.80s)
--- PASS: TestAccAzureRMLogAnalyticsWorkspaceLinkedService_complete (101.06s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	102.184s

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.

otherwise LGTM 👍

Type: schema.TypeString,
Required: true,
ForceNew: true,
DiffSuppressFunc: suppress.CaseDifference,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should be able to remove this, and instead update the validation to require lower-case here?

Copy link
Contributor

Choose a reason for hiding this comment

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

(we should also raise a bug about this API being broken, if there's not one already)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That would potentially break existing terraform and not match the portal. Workspace's are allowed to have capitalized characters and that resource works just fine with them (see updated tests)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

issue opened

@ghost ghost added size/L and removed size/M labels Jan 5, 2019
@katbyte
Copy link
Collaborator Author

katbyte commented Jan 5, 2019

tests:

==> Fixing source code with gofmt...
gofmt -s -w ./azurerm
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./azurerm -v -test.run=TestAccAzureRMLogAnalytics -timeout 180m -ldflags="-X=github.com/terraform-providers/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccAzureRMLogAnalyticsSolution_basicContainerMonitoring
=== PAUSE TestAccAzureRMLogAnalyticsSolution_basicContainerMonitoring
=== RUN   TestAccAzureRMLogAnalyticsSolution_basicSecurity
=== PAUSE TestAccAzureRMLogAnalyticsSolution_basicSecurity
=== RUN   TestAccAzureRMLogAnalyticsWorkspaceLinkedService_basic
=== PAUSE TestAccAzureRMLogAnalyticsWorkspaceLinkedService_basic
=== RUN   TestAccAzureRMLogAnalyticsWorkspaceLinkedService_complete
=== PAUSE TestAccAzureRMLogAnalyticsWorkspaceLinkedService_complete
=== RUN   TestAccAzureRMLogAnalyticsWorkspace_basic
=== PAUSE TestAccAzureRMLogAnalyticsWorkspace_basic
=== RUN   TestAccAzureRMLogAnalyticsWorkspace_complete
=== PAUSE TestAccAzureRMLogAnalyticsWorkspace_complete
=== CONT  TestAccAzureRMLogAnalyticsSolution_basicContainerMonitoring
=== CONT  TestAccAzureRMLogAnalyticsWorkspace_basic
=== CONT  TestAccAzureRMLogAnalyticsWorkspaceLinkedService_basic
=== CONT  TestAccAzureRMLogAnalyticsSolution_basicSecurity
=== CONT  TestAccAzureRMLogAnalyticsWorkspaceLinkedService_complete
=== CONT  TestAccAzureRMLogAnalyticsWorkspace_complete
--- PASS: TestAccAzureRMLogAnalyticsWorkspace_basic (89.13s)
--- PASS: TestAccAzureRMLogAnalyticsWorkspace_complete (89.38s)
--- PASS: TestAccAzureRMLogAnalyticsWorkspaceLinkedService_basic (93.87s)
--- PASS: TestAccAzureRMLogAnalyticsSolution_basicContainerMonitoring (95.03s)
--- PASS: TestAccAzureRMLogAnalyticsWorkspaceLinkedService_complete (159.70s)
--- PASS: TestAccAzureRMLogAnalyticsSolution_basicSecurity (160.89s)
PASS
ok  	github.com/terraform-providers/terraform-provider-azurerm/azurerm	162.547s

@katbyte katbyte merged commit 82613e1 into master Jan 5, 2019
@katbyte katbyte deleted the log-analytics/uppercase branch January 5, 2019 23:28
katbyte added a commit that referenced this pull request Jan 5, 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 👉 hashibot-feedback@hashicorp.com. 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.

automation log_analytics_workspace_linked_service forces new resource with no changes
2 participants