potiuk commented on a change in pull request #13585:
URL: https://github.com/apache/airflow/pull/13585#discussion_r554425542
##########
File path: chart/templates/_helpers.yaml
##########
@@ -357,6 +357,10 @@ server_tls_key_file = /etc/pgbouncer/server.key
{{ (printf "%s/config/airflow_local_settings.py" .Values.airflowHome) | quote
}}
{{- end }}
+{{ define "airflow_install_requirements_path" -}}
Review comment:
The name should be change to match the naming we already have.
In our Dockerfile we already have ADDITIONAL_PYTHON_DEPS variable and we are
installing additional python dependencies. @mik-laj rightfully noticed that not
alll deps can be installed this way (we do not have build-essentials in the
image for one) but some can be, and I agree it's nice to have, them, it should
be clear however, that those are additional dependencies, and they are not
de-facto "install_requirements". The "install_requirements" in python world
have a very precise meaning - those are to be found in `setup.py` or
`setup.cfg` and they mean "those need to be installed BEFORE you install
airflow". What we are talking about here are "additional python dependencies"
which are installed AFTER airflow is installed. So let's name it this way.
Relevant snippet from Dockerfile:
```
# Add extra python dependencies
ARG ADDITIONAL_PYTHON_DEPS=""
ENV ADDITIONAL_PYTHON_DEPS=${ADDITIONAL_PYTHON_DEPS}
```
----------------------------------------------------------------
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]
