BasPH commented on code in PR #29078: URL: https://github.com/apache/airflow/pull/29078#discussion_r1083850885
########## docs/helm-chart/index.rst: ########## @@ -140,3 +140,18 @@ will not start as the migrations will not be run: This is so these CI/CD services can perform updates without issues and preserve the immutability of Kubernetes Job manifests. This also applies if you install the chart using ``--wait`` in your ``helm install`` command. + +.. note:: + While deploying this Helm chart with Argo, you might encounter issues with database migrations not running automatically on upgrade. + +To ensure database migrations with Argo CD, you will need to add: + +.. code-block::yaml Review Comment: ```suggestion .. code-block:: yaml ``` RST won't display this code block without a whitespace in between `::` and `yaml`. ########## docs/helm-chart/index.rst: ########## @@ -140,3 +140,18 @@ will not start as the migrations will not be run: This is so these CI/CD services can perform updates without issues and preserve the immutability of Kubernetes Job manifests. This also applies if you install the chart using ``--wait`` in your ``helm install`` command. + +.. note:: + While deploying this Helm chart with Argo, you might encounter issues with database migrations not running automatically on upgrade. + +To ensure database migrations with Argo CD, you will need to add: + +.. code-block::yaml + + migrateDatabaseJob: + jobAnnotations: + "argocd.argoproj.io/hook": Sync + +This will ensure database migrations run when the Airflow Docker image is upgraded. This approach has a limitation in that the database migrations will run every time there is a ``Sync`` event in Argo. This is a trade-off for automation at the cost of some computational loss. Review Comment: ```suggestion This will run database migrations when the Airflow Docker image is upgraded. This approach has a limitation in that the database migrations will run every time there is a ``Sync`` event in Argo. This is a trade-off for automation at the cost of some computational loss. ``` Avoid double usage of "ensure database migrations", reads a bit nicer. ########## docs/helm-chart/index.rst: ########## @@ -140,3 +140,18 @@ will not start as the migrations will not be run: This is so these CI/CD services can perform updates without issues and preserve the immutability of Kubernetes Job manifests. This also applies if you install the chart using ``--wait`` in your ``helm install`` command. + +.. note:: + While deploying this Helm chart with Argo, you might encounter issues with database migrations not running automatically on upgrade. + +To ensure database migrations with Argo CD, you will need to add: + +.. code-block::yaml + + migrateDatabaseJob: + jobAnnotations: + "argocd.argoproj.io/hook": Sync + +This will ensure database migrations run when the Airflow Docker image is upgraded. This approach has a limitation in that the database migrations will run every time there is a ``Sync`` event in Argo. This is a trade-off for automation at the cost of some computational loss. + +If you use the Celery(Kubernetes)Executor, and using the built-in Redis, it is recommended that you setup a static Redis password either by supplying ``redis.passwordSecretName`` and ``redis.data.brokerUrlSecretName`` or ``redis.password``. See `Celery Backend <production-guide#celery-backend>`__ for more information about managing Celery Backend Review Comment: ```suggestion If you use the Celery(Kubernetes)Executor with the built-in Redis, it is recommended that you set up a static Redis password either by supplying ``redis.passwordSecretName`` and ``redis.data.brokerUrlSecretName`` or ``redis.password``. ``` Minor spelling nitpicks. Also, would remove the last reference. That links to a section about managing your own Celery Backend, while the prior sentence speaks about managing the built-in Redis. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
