jedcunningham commented on code in PR #29078:
URL: https://github.com/apache/airflow/pull/29078#discussion_r1084232512


##########
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:

Review Comment:
   ```suggestion
   To run database migrations with Argo CD automatically, you will need to add:
   ```
   
   nit



##########
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 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.

Review Comment:
   ```suggestion
   This will run database migrations every time there is a ``Sync`` event in 
Argo CD. While it is not ideal to run the migrations on every sync, it is a 
tradeoff that allows them to be run automatically.
   ```
   
   Maybe something like this?
   
   I think we could probably do without this, as this is the same situation 
with the helm hook. Though this might be my maintainer bias speaking here, so 
happy to leave it.



-- 
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: commits-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to