XD-DENG commented on a change in pull request #14766:
URL: https://github.com/apache/airflow/pull/14766#discussion_r593883854
##########
File path: chart/templates/check-values.yaml
##########
@@ -21,7 +21,18 @@ The sole purpose of this yaml file is it to check the values
file is consistent
{{- /*
##############################
- Redis related checks
+ Executors related checks
+#############################
+*/ -}}
+ {{- $supportedExecutors := list "LocalExecutor" "CeleryExecutor"
"KubernetesExecutor" "CeleryKubernetesExecutor" }}
+
+ {{- if not (has .Values.executor $supportedExecutors) }}
+ {{ required "Unsupported executor. Supported values: LocalExecutor,
CeleryExecutor, KubernetesExecutor, CeleryKubernetesExecutor." nil }}
Review comment:
Can we reuse the variable `supportedExecutors`? So that avoid
hard-coding the same thing.
##########
File path: chart/templates/scheduler/scheduler-networkpolicy.yaml
##########
@@ -40,7 +40,7 @@ spec:
release: {{ .Release.Name }}
policyTypes:
- Ingress
-{{- if (or (eq .Values.executor "LocalExecutor") (eq .Values.executor
"SequentialExecutor")) }}
+{{- if (eq .Values.executor "LocalExecutor") }}
Review comment:
```suggestion
{{- if eq .Values.executor "LocalExecutor" }}
```
##########
File path: docs/helm-chart/index.rst
##########
@@ -42,12 +42,29 @@ This chart will bootstrap an `Airflow
<https://airflow.apache.org>`__
deployment on a `Kubernetes <http://kubernetes.io>`__ cluster using the
`Helm <https://helm.sh>`__ package manager.
-Prerequisites
--------------
+Requirements
+------------
- Kubernetes 1.14+ cluster
- Helm 2.11+ or Helm 3.0+
-- PV provisioner support in the underlying infrastructure
+- PV provisioner support in the underlying infrastructure (optionally)
+
+Features
+--------
+
+* Supported executors: ``LocalExecutor``, ``CeleryExecutor``,
``CeleryKubernetesExecutor``, ``KubernetesExecutor``.
+* Supported Airflow version: ``1.10+``, ``2.0+``
+* Supported database backend: ``PostgresSQL``, ``MySQL``
+* Autoscaling for ``CeleryExecutor`` provided by KEDA
+* PostgresSQL and PGBouncer with a battle-tested configuration
Review comment:
```suggestion
* PostgresSQL and PgBouncer with a battle-tested configuration
```
nit
##########
File path: chart/templates/scheduler/scheduler-service.yaml
##########
@@ -18,7 +18,7 @@
################################
## Airflow Scheduler Service
#################################
-{{- if (or (eq .Values.executor "LocalExecutor") (eq .Values.executor
"SequentialExecutor")) }}
+{{- if (eq .Values.executor "LocalExecutor") }}
Review comment:
```suggestion
{{- if eq .Values.executor "LocalExecutor" }}
```
##########
File path: chart/templates/rbac/pod-launcher-rolebinding.yaml
##########
@@ -19,8 +19,7 @@
## Airflow Pod Launcher Role Binding
#################################
{{- if and .Values.rbacEnabled .Values.allowPodLaunching }}
-{{- $schedulerLaunchExecutors := list "LocalExecutor" "SequentialExecutor"
"KubernetesExecutor" "CeleryKubernetesExecutor" }}
-{{- $workerLaunchExecutors := list "CeleryExecutor" "KubernetesExecutor"
"CeleryKubernetesExecutor" }}
+{{- $podLaunchExecutors := list "LocalExecutor" "KubernetesExecutor"
"CeleryKubernetesExecutor" }}
Review comment:
I may have missed or misunderstood something: may you please help
clarify why `CeleryExecutor` is not in this list?
##########
File path: chart/README.md
##########
@@ -28,11 +28,27 @@
This chart will bootstrap an [Airflow](https://airflow.apache.org) deployment
on a [Kubernetes](http://kubernetes.io)
cluster using the [Helm](https://helm.sh) package manager.
-## Prerequisites
+## Requirements
- Kubernetes 1.14+ cluster
- Helm 2.11+ or Helm 3.0+
-- PV provisioner support in the underlying infrastructure
+- PV provisioner support in the underlying infrastructure (optionally)
+
+## Features
+
+* Supported executors: ``LocalExecutor``, ``CeleryExecutor``,
``CeleryKubernetesExecutor``, ``KubernetesExecutor``.
+* Supported Airflow version: ``1.10+``, ``2.0+``
+* Supported database backend: ``PostgresSQL``, ``MySQL``
+* Autoscaling for ``CeleryExecutor`` provided by KEDA
+* PostgresSQL and PGBouncer with a battle-tested configuration
Review comment:
```suggestion
* PostgresSQL and PgBouncer with a battle-tested configuration
```
nit
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
