azurerm_container_app_environment - Make log_analytics_workspace_id an optional argument.
#22733
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
service/container-apps
size/M
documentation
waiting-response
labels
1 task
stephybun
requested changes
Aug 7, 2023
| @@ -25,7 +25,7 @@ func TestAccContainerAppEnvironment_basic(t *testing.T) { | |||
|
|
|||
| data.ResourceTest(t, r, []acceptance.TestStep{ | |||
| { | |||
| Config: r.basic(data), | |||
| Config: r.basic(data, true), | |||
There was a problem hiding this comment.
Instead up updating the config to take an argument can you remove the property from basic and add it to the complete test instead.
| @@ -44,7 +44,7 @@ The following arguments are supported: | |||
|
|
|||
| * `location` - (Required) Specifies the supported Azure location where the Container App Environment is to exist. Changing this forces a new resource to be created. | |||
|
|
|||
| * `log_analytics_workspace_id` - (Required) The ID for the Log Analytics Workspace to link this Container Apps Managed Environment to. Changing this forces a new resource to be created. | |||
| * `log_analytics_workspace_id` - (Optional) The ID for the Log Analytics Workspace to link this Container Apps Managed Environment to. Changing this forces a new resource to be created. | |||
There was a problem hiding this comment.
This would need to be moved down to the section where we list the Optional properties.
|
@stephybun Thanks for your suggestions, I've updated this pr, would you please give it another review? Thanks! |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We found that
log_analytics_workspace_idis not required forazurerm_container_app_environmentanymore, this pr change it to an optional argument so our users could save cost. This pr should solve #20748.The newly added e2e test passed on my side: