Skip to content

Conversation

@EricGao888
Copy link
Member

Currently, some comments in worker-kedaautoscaler.yaml are inconsistent with corresponding default values in values.yaml. This PR corrects those comments.

pollingInterval: {{ .Values.workers.keda.pollingInterval }} # Optional. Default: 30 seconds
cooldownPeriod: {{ .Values.workers.keda.cooldownPeriod }} # Optional. Default: 300 seconds
minReplicaCount: {{ .Values.workers.keda.minReplicaCount }} # Optional. Default: 0
maxReplicaCount: {{ .Values.workers.keda.maxReplicaCount }} # Optional. Default: 100

airflow/chart/values.yaml

Lines 498 to 513 in 493b433

keda:
enabled: false
namespaceLabels: {}
# How often KEDA polls the airflow DB to report new scale requests to the HPA
pollingInterval: 5
# How many seconds KEDA will wait before scaling to zero.
# Note that HPA has a separate cooldown period for scale-downs
cooldownPeriod: 30
# Minimum number of workers created by keda
minReplicaCount: 0
# Maximum number of workers created by keda
maxReplicaCount: 10

@boring-cyborg boring-cyborg bot added the area:helm-chart Airflow Helm Chart label Jan 11, 2023
Comment on lines 40 to 41
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pollingInterval: {{ .Values.workers.keda.pollingInterval }} # Optional. Default: 5 seconds
cooldownPeriod: {{ .Values.workers.keda.cooldownPeriod }} # Optional. Default: 30 seconds
pollingInterval: {{ .Values.workers.keda.pollingInterval }}
cooldownPeriod: {{ .Values.workers.keda.cooldownPeriod }}

Let's actually just remove it completely - we don't really do this elsewhere and they are just asking to become out of sync.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, thanks for the suggestions : )

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
maxReplicaCount: {{ .Values.workers.keda.maxReplicaCount }} # Optional. Default: 10
maxReplicaCount: {{ .Values.workers.keda.maxReplicaCount }}

@EricGao888 EricGao888 force-pushed the Fix-incorrect-comments-in-kedaautoscaler branch from d1a885f to 598a2f3 Compare January 12, 2023 03:48
@EricGao888 EricGao888 closed this Jan 12, 2023
@EricGao888 EricGao888 reopened this Jan 12, 2023
@EricGao888 EricGao888 requested review from jedcunningham and removed request for dstandish January 12, 2023 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:helm-chart Airflow Helm Chart

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants