-
Notifications
You must be signed in to change notification settings - Fork 571
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
Avoid continuous destroy and create of azapi_resource diag_settings #952
Conversation
@microsoft-github-policy-service agree
@microsoft-github-policy-service agree |
Hi! Do you think we should use the default location that is supplied as an input already? is there a case for them being different? |
variable "diag_settings_location" { | ||
type = string | ||
description = "Will set the Azure region in which region azapi diagnostic settings will be deployed. Please see: https://azure.microsoft.com/en-gb/global-infrastructure/geographies/" | ||
default = null | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this and use default location please?
resource "azapi_resource" "diag_settings" { | ||
for_each = local.azapi_mg_diagnostics | ||
type = "Microsoft.Insights/diagnosticSettings@2021-05-01-preview" | ||
name = "toLA" | ||
location = local.diag_settings_location |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please can we use default location
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes you can use default_location
@@ -39,6 +39,7 @@ locals { | |||
module.management_resources.configuration.template_file_variables, | |||
) | |||
default_location = lower(var.default_location) | |||
diag_settings_location = lower(var.diag_settings_location) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again please can we use default location?
Thank you for your PR! A few comments on the implementation but really appreciate the effort |
closed in favour of #968 thank you for your PR, I have taken the fix and ported it across |
Overview/Summary
Fixed the destroy and create of azapi_resource diag_settings. This PR is linked with the issue #939 .
This PR fixes/adds/changes/removes
As part of this Pull Request I have
main
branch