potiuk commented on a change in pull request #4937: [AIRFLOW-4116]
Multi-staging includes CI image [Step 2/3]
URL: https://github.com/apache/airflow/pull/4937#discussion_r289861478
##########
File path: Dockerfile
##########
@@ -125,16 +239,19 @@ RUN echo "Pip version: ${PIP_VERSION}"
RUN pip install --upgrade pip==${PIP_VERSION}
+# We are copying everything with airflow:airflow user:group even if we use
root to run the scripts
+# This is fine as root user will be able to use those dirs anyway.
+
# Airflow sources change frequently but dependency configuration won't change
that often
# We copy setup.py and other files needed to perform setup of dependencies
# This way cache here will only be invalidated if any of the
# version/setup configuration change but not when airflow sources change
-COPY --chown=airflow:airflow setup.py /opt/airflow/setup.py
-COPY --chown=airflow:airflow setup.cfg /opt/airflow/setup.cfg
+COPY --chown=airflow:airflow setup.py ${AIRFLOW_HOME}/setup.py
+COPY --chown=airflow:airflow setup.cfg ${AIRFLOW_HOME}/setup.cfg
-COPY --chown=airflow:airflow airflow/version.py /opt/airflow/airflow/version.py
-COPY --chown=airflow:airflow airflow/__init__.py
/opt/airflow/airflow/__init__.py
-COPY --chown=airflow:airflow airflow/bin/airflow
/opt/airflow/airflow/bin/airflow
+COPY --chown=airflow:airflow airflow/version.py
${AIRFLOW_HOME}/airflow/version.py
Review comment:
@ashb - as for the AIRFLOW_HOME (/home/airflow) vs. AIRFLOW_USER_HOME
(/opt/airflow)- this is what we currently have even in the current CI.
AIRFLOW_HOME is where the airflow sources are located and where the DAGS and
LOGS are eventually placed and where version.py and airflow/bin actually belong
I think.
AIRFLOW_USER_HOME (/home/airflow) is where the home directory of the airflow
user with all the .bash_profile scripts etc., and I don't think it's best place
for version.py etc..
They were two distinct locations even in the original CI scripts. The
sources were originally in /app and HOME was /home/airflow. It was not possible
to have them in one folder because the whole airflow sources directory was
mounted as /app folder (so it would overrride all the "home" scripts like
.bash_profile etc.:
From:
https://github.com/apache/airflow/blob/master/scripts/ci/docker-compose.yml
```
volumes:
- ../../:/app
```
Hovewer since you raised it - In the current setup we could combine those
two. Because in the new Docker-compose configuration I only selectively mount
only a subset of sources:
From:
https://github.com/PolideaInternal/airflow/blob/simplified-development-workflow/scripts/ci/docker-compose-local.yml
```
volumes:
- ../../airflow:/opt/airflow/airflow
- ../../logs:/opt/airflow/logs
- ../../dags:/opt/airflow/dags
- ../../dev:/opt/airflow/dev
- ../../docs:/opt/airflow/docs
- ../../scripts:/opt/airflow/scripts
- ../../tests:/opt/airflow/tests
....
```
So we could have AIRFLOW_HOME point to /home/airflow and have
/home/airflow/airflow, /home/airflow/dags etc.
Do you think that it is better approach this way ?
----------------------------------------------------------------
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]
With regards,
Apache Git Services
