[GitHub] [airflow] Taragolis commented on a diff in pull request #33879: Prevent users from misusing _PIP_ADDITIONAL_REQUIREMENTS

["Taragolis (via GitHub)" <gi...@apache.org>]

Taragolis commented on code in PR #33879:
URL: https://github.com/apache/airflow/pull/33879#discussion_r1308589948


##########
scripts/docker/entrypoint_prod.sh:
##########
@@ -296,10 +296,17 @@ if [[ -n "${_AIRFLOW_WWW_USER_CREATE=}" ]] ; then
 fi
 
 if [[ -n "${_PIP_ADDITIONAL_REQUIREMENTS=}" ]] ; then
+    COLOR_RED=$'\e[31m'
+    COLOR_RESET=$'\e[0m'
+    COLOR_YELLOW=$'\e[33m'
+
+    # The container will shutdown after that many seconds of use automatically when _PIP_ADDITIONAL_REQUIREMENTS ARE USED
+    KILL_TIMEOUT=600
+    >&2 echo
+    >&2 echo "${COLOR_YELLOW}Installing additional requirements: '${_PIP_ADDITIONAL_REQUIREMENTS}' ${COLOR_RESET}"
     >&2 echo
-    >&2 echo "!!!!!  Installing additional requirements: '${_PIP_ADDITIONAL_REQUIREMENTS}' !!!!!!!!!!!!"
+    >&2 echo "${COLOR_YELLOW}WARNING: This is a development/test feature only. NEVER use it in production!${COLOR_RESET}"

Review Comment:
   I'm not sure about **development/test feature only**, I would never recommend to use it in development (deploy into dev environment) or test (deploy into the test/qa/preproduction environment).
   
   I thought how it propagated into the prod: It work in local development, let's deploy on dev/test/staging



##########
docs/docker-stack/changelog.rst:
##########
@@ -58,6 +58,21 @@ here so that users affected can find the reason for the changes.
 Airflow 2.7
 ~~~~~~~~~~~
 
+* 2.7.2
+
+  * Several fixes to how ``__PIP_ADDITIONAL_REQUIREMENTS`` works:
+
+    * When installing additional requirements, ``airflow_version=<AIRFLOW_VERSION>`` is passed to the pip command. This ensures that the
+      airflow version is never accidentally downgraded or upgraded when installing incompatible requirements.
+
+    * Airflow runs ``pip check`` after installing the requirements which will fail if ``pip`` encounters incompatible dependencies after installation -
+      this might happen in case some earlier installed dependencies are in conflict with the newly installed ones. Failure of pip check will fail
+      the image.
+
+    * Setting __PIP_ADDITIONAL_REQUIREMENTS will cause the image to shutdown after 10 minutes. This has always been a development only feature

Review Comment:
   ```suggestion
       * Setting _PIP_ADDITIONAL_REQUIREMENTS will cause the image to shutdown after 10 minutes. This has always been a development only feature
   ```



##########
docs/docker-stack/changelog.rst:
##########
@@ -58,6 +58,21 @@ here so that users affected can find the reason for the changes.
 Airflow 2.7
 ~~~~~~~~~~~
 
+* 2.7.2
+
+  * Several fixes to how ``__PIP_ADDITIONAL_REQUIREMENTS`` works:

Review Comment:
   ```suggestion
     * Several fixes to how ``_PIP_ADDITIONAL_REQUIREMENTS`` works:
   ```



-- 
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-unsubscribe@airflow.apache.org

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