-
Notifications
You must be signed in to change notification settings - Fork 0
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
[CES-199] - Avoid modules dependencies from existing resources #144
base: main
Are you sure you want to change the base?
Conversation
|
Configuration for the target service (App Service or Function App) to which the autoscaler will be applied. | ||
|
||
For existing services: | ||
- Provide either 'app_service_name' or 'function_app_name'. |
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.
What about having an app service autoscaler module AND a function autoscaler module instead? you may consider to share most of the code but this configuration.
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.
another option is to something like this instead:
variable "target_service" {
type = object({
app_service = optional(object({
app_service_id = string
app_service_name = string
app_service_plan_id = string
location = string
}))
function_app = optional(object({
function_app_id = string
function_app_name = string
location = string
}))
})
validation {
condition = length([for v in [var.target_service.app_service, var.target_service.function_app] : v if v != null]) == 1
error_message = "You must define either app_service or function_app, but not both."
}
}
which will be a breaking change i guess
|
||
Note: Only one service type (App Service or Function App) should be specified. | ||
EOT | ||
|
||
validation { |
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.
see comment above to simplify this logic. I think this would be a breaking change anyway.
@@ -6,7 +6,8 @@ variable "principal_id" { | |||
variable "cosmos" { | |||
description = "A list of CosmosDB role assignments" | |||
type = list(object({ | |||
account_name = string | |||
account_name = optional(string) | |||
account_id = optional(string) |
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.
should be documented into var description
List of Event Hub configurations for role assignments. | ||
For existing Event Hubs: | ||
- Provide namespace_name and event_hub_names | ||
For new Event Hubs being created concurrently: |
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.
what do you mean here with "new Event Hubs being created concurrently"?
@@ -6,7 +6,8 @@ variable "principal_id" { | |||
variable "cosmos" { | |||
description = "A list of CosmosDB role assignments" | |||
type = list(object({ | |||
account_name = string | |||
account_name = optional(string) | |||
account_id = optional(string) |
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.
should be documented
List of changes
Motivation and context
Type of changes
Does this introduce a change to production resources with possible user impact?
Other information