You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/01/01 07:33:00 UTC
[GitHub] [airflow] potiuk opened a new pull request #13422: Removes pip download when installing from local packages
potiuk opened a new pull request #13422:
URL: https://github.com/apache/airflow/pull/13422
This PR improves building production image from local packages,
in preparation for moving provider requirements out of setup.cfg.
Previously `pip download` step was executed in the CI scripts
in order to download all the packages that were needed. However
this had two problems:
1) PIP download was executed outside of Dockerfile in CI scripts
which means that any change to requirements there could not
be executed in 'workflow_run' event - because main branch version
of CI scripts is used there. We want to add extra requirements
when installing airflow so in order to be able to change
it, those requirements should be added in Dockerfile.
This will be done in the follow-up #13409 PR.
2) Packages downloaded with PIP download have a "file" version
rather than regular == version when you run pip freeze/check.
This looks weird and while you can figure out the version
from file name, when you `pip install` them, they look
much more normal. The airflow package and provider package
will still get the "file" form but this is ok because we are
building those packages from sources and they are not yet
available in PyPI.
Example:
adal==1.2.5
aiohttp==3.7.3
alembic==1.4.3
amqp==2.6.1
apache-airflow @ file:///docker-context-files/apache_airflow-2.1.0.dev0-py3-none-any.whl
apache-airflow-providers-amazon @ file:///docker-context-files/apache_airflow_providers_amazon-1.0.0-py3-none-any.whl
apache-airflow-providers-celery @ file:///docker-context-files/apache_airflow_providers_celery-1.0.0-py3-none-any.whl
...
With this PR, we do not `pip download` all packages, but instead
we prepare airflow + providers packages as .whl files and
install them from there (all the dependencies are installed
from PyPI)
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#issuecomment-753282743
[The Workflow run](https://github.com/apache/airflow/actions/runs/455805748) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #13422:
URL: https://github.com/apache/airflow/pull/13422
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550817968
##########
File path: Dockerfile
##########
@@ -247,31 +247,54 @@ ENV UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
WORKDIR /opt/airflow
-# remove mysql from extras if client is not installed
+# hadolint ignore=SC2086, SC2010
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
+ # Remove mysql from extras if client is not installed \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--upgrade --upgrade-strategy eager; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
Review comment:
I prefer to leave it as is unless you have better proposa.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#issuecomment-753315005
[The Workflow run](https://github.com/apache/airflow/actions/runs/456171910) is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#issuecomment-753319204
I think this one should be good to go! I need to merge it before #13409
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550768525
##########
File path: Dockerfile
##########
@@ -247,31 +247,54 @@ ENV UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
WORKDIR /opt/airflow
-# remove mysql from extras if client is not installed
+# hadolint ignore=SC2086, SC2010
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
+ # Remove mysql from extras if client is not installed \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--upgrade --upgrade-strategy eager; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
else \
- pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
+ pip install --upgrade --upgrade-strategy only-if-needed \
Review comment:
Now - we add "only-if needed " strategy here in case someone changes setup.py requirements and the pre-installed versions no longer fit in those requirements.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550768639
##########
File path: Dockerfile
##########
@@ -247,31 +247,54 @@ ENV UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
WORKDIR /opt/airflow
-# remove mysql from extras if client is not installed
+# hadolint ignore=SC2086, SC2010
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
+ # Remove mysql from extras if client is not installed \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--upgrade --upgrade-strategy eager; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
else \
- pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
+ pip install --upgrade --upgrade-strategy only-if-needed \
+ --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
+ fi; \
+ fi; \
+ if [[ ${INSTALL_FROM_DOCKER_CONTEXT_FILES} == "true" ]]; then \
+ reinstalling_apache_airflow_packages=$(ls /docker-context-files/apache?airflow*.{whl,tar.gz} 2>/dev/null || true); \
Review comment:
This is an interesting one. Instead of `pip download` as we did before, we want to install all dependencies here for airflow packages and follow the constraints we already have (unless we upgrade to newer dependencies -then we do not use constraints).
This is the case where we haave only "airflow" + "airflow packages" build from sources to install (which happens on CI).
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#issuecomment-753413053
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#issuecomment-753385195
It's green! seems that the "reverse lookup disabling" solves the problem and we are all green!
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550768356
##########
File path: Dockerfile
##########
@@ -247,31 +247,54 @@ ENV UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
WORKDIR /opt/airflow
-# remove mysql from extras if client is not installed
+# hadolint ignore=SC2086, SC2010
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
+ # Remove mysql from extras if client is not installed \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--upgrade --upgrade-strategy eager; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
Review comment:
Here we resore PIP to the version specified (eager upgrade will upgrade 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550769201
##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -913,31 +913,40 @@ function build_images::determine_docker_cache_strategy() {
}
-function build_images::build_prod_images_from_packages() {
+function build_image::assert_variable() {
+ local variable_name="${1}"
+ local expected_value="${2}"
+ local variable_value=${!variable_name}
+ if [[ ${variable_value} != "${expected_value}" ]]; then
+ echo
+ echo "${COLOR_RED_ERROR}: Variable ${variable_name}: expected_value: '${expected_value}' but was '${variable_value}'!${COLOR_RESET}"
+ echo
+ exit 1
+ fi
+}
+
+function build_images::build_prod_images_from_locally_built_airflow_packages() {
+ # We do not install from PyPI
+ build_image::assert_variable INSTALL_FROM_PYPI "false"
+ # But then we reinstall airflow and providers from prepared packages in the docker context files
+ build_image::assert_variable INSTALL_FROM_DOCKER_CONTEXT_FILES "true"
+ # But we install everything from scratch to make a "clean" installation in case any dependencies got removed
+ build_image::assert_variable AIRFLOW_PRE_CACHED_PIP_PACKAGES "false"
+
# Cleanup dist and docker-context-files folders
mkdir -pv "${AIRFLOW_SOURCES}/dist"
mkdir -pv "${AIRFLOW_SOURCES}/docker-context-files"
rm -f "${AIRFLOW_SOURCES}/dist/"*.{whl,tar.gz}
rm -f "${AIRFLOW_SOURCES}/docker-context-files/"*.{whl,tar.gz}
- runs::run_pip_download
Review comment:
We do not need to run pip download now. This is good because this code is run from master, so previously we could not change the requirements for that pip download. In the #13409 we are going to add per-image requirements and since they will be in the images, not scripts, they are safe to run in `workflow_run`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550824560
##########
File path: Dockerfile
##########
@@ -247,31 +247,54 @@ ENV UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
WORKDIR /opt/airflow
-# remove mysql from extras if client is not installed
+# hadolint ignore=SC2086, SC2010
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
+ # Remove mysql from extras if client is not installed \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--upgrade --upgrade-strategy eager; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
Review comment:
In the first comment, I added an example. To show my idea better, I prepared the entire commit. Does this solve this problem?
```diff
From cc68b6ba19a4bc11e5aed3ec01f3e4f86f76f745 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kamil=20Bregu=C5=82a?= <ka...@polidea.com>
Date: Sat, 2 Jan 2021 01:16:03 +0100
Subject: Add constraints to prevent upgrade of pip
---
Dockerfile | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/Dockerfile b/Dockerfile
index 81737a15b..b72d859dc 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -255,13 +255,13 @@ RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
+ --constraint <(echo "pip==${AIRFLOW_PIP_VERSION}") \
--upgrade --upgrade-strategy eager; \
- pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
else \
pip install --upgrade --upgrade-strategy only-if-needed \
--user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
+ --constraint <(echo "pip==${AIRFLOW_PIP_VERSION}") \
--constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
- pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
fi; \
fi; \
if [[ ${INSTALL_FROM_DOCKER_CONTEXT_FILES} == "true" ]]; then \
@@ -270,12 +270,13 @@ RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
if [[ "${reinstalling_apache_airflow_packages}" != "" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --force-reinstall --upgrade --upgrade-strategy eager \
+ --constraint <(echo "pip==${AIRFLOW_PIP_VERSION}") \
--user ${reinstalling_apache_airflow_packages}; \
- pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
else \
pip install --force-reinstall --upgrade --upgrade-strategy only-if-needed \
- --user ${reinstalling_apache_airflow_packages} --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
- pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
+ --user ${reinstalling_apache_airflow_packages} \
+ --constraint <(echo "pip==${AIRFLOW_PIP_VERSION}") \
+ --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
fi; \
fi ; \
# All the others we want to reinstall as-is, without dependencies \
@@ -287,12 +288,14 @@ RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
fi; \
if [[ -n "${ADDITIONAL_PYTHON_DEPS}" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
- pip install --user ${ADDITIONAL_PYTHON_DEPS} --upgrade --upgrade-strategy eager; \
- pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
+ pip install --user ${ADDITIONAL_PYTHON_DEPS} \
+ --constraint <(echo "pip==${AIRFLOW_PIP_VERSION}") \
+ --upgrade --upgrade-strategy eager; \
else \
- pip install --user ${ADDITIONAL_PYTHON_DEPS} --upgrade --upgrade-strategy only-if-needed \
+ pip install --user ${ADDITIONAL_PYTHON_DEPS} \
+ --upgrade --upgrade-strategy only-if-needed \
+ --constraint <(echo "pip==${AIRFLOW_PIP_VERSION}") \
--constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
- pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
fi; \
fi; \
find /root/.local/ -name '*.pyc' -print0 | xargs -0 rm -r || true ; \
--
2.28.0
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550768639
##########
File path: Dockerfile
##########
@@ -247,31 +247,54 @@ ENV UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
WORKDIR /opt/airflow
-# remove mysql from extras if client is not installed
+# hadolint ignore=SC2086, SC2010
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
+ # Remove mysql from extras if client is not installed \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--upgrade --upgrade-strategy eager; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
else \
- pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
+ pip install --upgrade --upgrade-strategy only-if-needed \
+ --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
+ fi; \
+ fi; \
+ if [[ ${INSTALL_FROM_DOCKER_CONTEXT_FILES} == "true" ]]; then \
+ reinstalling_apache_airflow_packages=$(ls /docker-context-files/apache?airflow*.{whl,tar.gz} 2>/dev/null || true); \
Review comment:
This is an interesting one. Instead of `pip download` as we did before, we want to install all dependencies here for airflow packages and follow the constraints we already have (unless we upgrade to newer dependencies).
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550817892
##########
File path: Dockerfile
##########
@@ -247,31 +247,54 @@ ENV UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
WORKDIR /opt/airflow
-# remove mysql from extras if client is not installed
+# hadolint ignore=SC2086, SC2010
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
+ # Remove mysql from extras if client is not installed \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--upgrade --upgrade-strategy eager; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
Review comment:
What update do you suggest?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550824560
##########
File path: Dockerfile
##########
@@ -247,31 +247,54 @@ ENV UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
WORKDIR /opt/airflow
-# remove mysql from extras if client is not installed
+# hadolint ignore=SC2086, SC2010
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
+ # Remove mysql from extras if client is not installed \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--upgrade --upgrade-strategy eager; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
Review comment:
In the first comment, I added an example. To show my idea better, I prepared the entire commit. Does this solve this problem?
```
From cc68b6ba19a4bc11e5aed3ec01f3e4f86f76f745 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Kamil=20Bregu=C5=82a?= <ka...@polidea.com>
Date: Sat, 2 Jan 2021 01:16:03 +0100
Subject: Add constraints to prevent upgrade of pip
---
Dockerfile | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/Dockerfile b/Dockerfile
index 81737a15b..b72d859dc 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -255,13 +255,13 @@ RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
+ --constraint <(echo "pip==${AIRFLOW_PIP_VERSION}") \
--upgrade --upgrade-strategy eager; \
- pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
else \
pip install --upgrade --upgrade-strategy only-if-needed \
--user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
+ --constraint <(echo "pip==${AIRFLOW_PIP_VERSION}") \
--constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
- pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
fi; \
fi; \
if [[ ${INSTALL_FROM_DOCKER_CONTEXT_FILES} == "true" ]]; then \
@@ -270,12 +270,13 @@ RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
if [[ "${reinstalling_apache_airflow_packages}" != "" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --force-reinstall --upgrade --upgrade-strategy eager \
+ --constraint <(echo "pip==${AIRFLOW_PIP_VERSION}") \
--user ${reinstalling_apache_airflow_packages}; \
- pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
else \
pip install --force-reinstall --upgrade --upgrade-strategy only-if-needed \
- --user ${reinstalling_apache_airflow_packages} --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
- pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
+ --user ${reinstalling_apache_airflow_packages} \
+ --constraint <(echo "pip==${AIRFLOW_PIP_VERSION}") \
+ --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
fi; \
fi ; \
# All the others we want to reinstall as-is, without dependencies \
@@ -287,12 +288,14 @@ RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
fi; \
if [[ -n "${ADDITIONAL_PYTHON_DEPS}" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
- pip install --user ${ADDITIONAL_PYTHON_DEPS} --upgrade --upgrade-strategy eager; \
- pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
+ pip install --user ${ADDITIONAL_PYTHON_DEPS} \
+ --constraint <(echo "pip==${AIRFLOW_PIP_VERSION}") \
+ --upgrade --upgrade-strategy eager; \
else \
- pip install --user ${ADDITIONAL_PYTHON_DEPS} --upgrade --upgrade-strategy only-if-needed \
+ pip install --user ${ADDITIONAL_PYTHON_DEPS} \
+ --upgrade --upgrade-strategy only-if-needed \
+ --constraint <(echo "pip==${AIRFLOW_PIP_VERSION}") \
--constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
- pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
fi; \
fi; \
find /root/.local/ -name '*.pyc' -print0 | xargs -0 rm -r || true ; \
--
2.28.0
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550768639
##########
File path: Dockerfile
##########
@@ -247,31 +247,54 @@ ENV UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
WORKDIR /opt/airflow
-# remove mysql from extras if client is not installed
+# hadolint ignore=SC2086, SC2010
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
+ # Remove mysql from extras if client is not installed \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--upgrade --upgrade-strategy eager; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
else \
- pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
+ pip install --upgrade --upgrade-strategy only-if-needed \
+ --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
+ fi; \
+ fi; \
+ if [[ ${INSTALL_FROM_DOCKER_CONTEXT_FILES} == "true" ]]; then \
+ reinstalling_apache_airflow_packages=$(ls /docker-context-files/apache?airflow*.{whl,tar.gz} 2>/dev/null || true); \
Review comment:
This is an interesting one. Instead of `pip download` as we did before, we want to install all dependencies here for airflow packages and follow the constraints we already have (unless we upgrade to newer dependencies).
This is the case where we haave only "airflow" + "airflow packages" build from sources to install (which happens on CI).
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550817968
##########
File path: Dockerfile
##########
@@ -247,31 +247,54 @@ ENV UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
WORKDIR /opt/airflow
-# remove mysql from extras if client is not installed
+# hadolint ignore=SC2086, SC2010
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
+ # Remove mysql from extras if client is not installed \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--upgrade --upgrade-strategy eager; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
Review comment:
I prefer to leave it as is unless you have better proposal
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550768835
##########
File path: Dockerfile
##########
@@ -247,31 +247,54 @@ ENV UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
WORKDIR /opt/airflow
-# remove mysql from extras if client is not installed
+# hadolint ignore=SC2086, SC2010
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
+ # Remove mysql from extras if client is not installed \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--upgrade --upgrade-strategy eager; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
else \
- pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
+ pip install --upgrade --upgrade-strategy only-if-needed \
+ --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
+ fi; \
+ fi; \
+ if [[ ${INSTALL_FROM_DOCKER_CONTEXT_FILES} == "true" ]]; then \
+ reinstalling_apache_airflow_packages=$(ls /docker-context-files/apache?airflow*.{whl,tar.gz} 2>/dev/null || true); \
+ # We want to install apache airflow packages with constraints \
+ if [[ "${reinstalling_apache_airflow_packages}" != "" ]]; then \
+ if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
+ pip install --force-reinstall --upgrade --upgrade-strategy eager \
+ --user ${reinstalling_apache_airflow_packages}; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
+ else \
+ pip install --force-reinstall --upgrade --upgrade-strategy only-if-needed \
+ --user ${reinstalling_apache_airflow_packages} --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
+ fi; \
+ fi ; \
+ # All the others we want to reinstall as-is, without dependencies \
+ reinstalling_other_packages=$(ls /docker-context-files/*.{whl,tar.gz} 2>/dev/null | \
Review comment:
But for all other packages, we want to just install whatever we have there without dependencies. If the packages are result of `pip download`, this serves the purpose of 'offline installation" mechanism.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550826713
##########
File path: Dockerfile
##########
@@ -247,31 +247,54 @@ ENV UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
WORKDIR /opt/airflow
-# remove mysql from extras if client is not installed
+# hadolint ignore=SC2086, SC2010
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
+ # Remove mysql from extras if client is not installed \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--upgrade --upgrade-strategy eager; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
Review comment:
I think it might not solve it. I have no 100% certainly but `--constraint` is not really binding (at least for pip 20.2.4). When you are upgrading with `eager` strategy, PIP often installs dependencies which are not within constraints. Constraint is merely a "hint" which does not have to be followed (and especially with 'eager' upgrade strategy.
I think I prefer to explicitly force pip version by installing it explicitly after.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550814365
##########
File path: Dockerfile
##########
@@ -247,31 +247,54 @@ ENV UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
WORKDIR /opt/airflow
-# remove mysql from extras if client is not installed
+# hadolint ignore=SC2086, SC2010
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
+ # Remove mysql from extras if client is not installed \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--upgrade --upgrade-strategy eager; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
Review comment:
Can we add this limitation to the previous command so that this update never happens?
```bash
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--constraint <<(echo "pip==${AIRFLOW_PIP_VERSION}") \
--upgrade --upgrade-strategy eager; \
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#issuecomment-753285985
[The Workflow run](https://github.com/apache/airflow/actions/runs/455842635) is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550768954
##########
File path: Dockerfile.ci
##########
@@ -325,7 +325,8 @@ RUN if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
pip install -e ".[${AIRFLOW_EXTRAS}]" --upgrade --upgrade-strategy eager; \
pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
else \
- pip install -e ".[${AIRFLOW_EXTRAS}]" --upgrade --upgrade-strategy only-if-needed; \
+ pip install -e ".[${AIRFLOW_EXTRAS}]" --upgrade --upgrade-strategy only-if-needed\
+ --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
Review comment:
We want to stick to the constraints here as well (unless setup.py requirements change)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550769052
##########
File path: Dockerfile.ci
##########
@@ -334,11 +335,28 @@ RUN if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
# they are also installed additionally to whatever is installed from Airflow.
COPY docker-context-files/ /docker-context-files/
-RUN if [[ ${INSTALL_FROM_DOCKER_CONTEXT_FILES} != "true" ]]; then \
- if ls /docker-context-files/*.{whl,tar.gz} 1> /dev/null 2>&1; then \
- pip install --no-deps /docker-context-files/*.{whl,tar.gz}; \
+# hadolint ignore=SC2086, SC2010
Review comment:
Same 'docker-file-context' installation for CI image. It might be useful only in case we want to test CI image built from packages, good to keep that option.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a change in pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#discussion_r550768835
##########
File path: Dockerfile
##########
@@ -247,31 +247,54 @@ ENV UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
WORKDIR /opt/airflow
-# remove mysql from extras if client is not installed
+# hadolint ignore=SC2086, SC2010
RUN if [[ ${INSTALL_MYSQL_CLIENT} != "true" ]]; then \
+ # Remove mysql from extras if client is not installed \
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/mysql,}; \
fi; \
if [[ ${INSTALL_FROM_PYPI} == "true" ]]; then \
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--upgrade --upgrade-strategy eager; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
else \
- pip install --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
+ pip install --upgrade --upgrade-strategy only-if-needed \
+ --user "${AIRFLOW_INSTALLATION_METHOD}[${AIRFLOW_EXTRAS}]${AIRFLOW_INSTALL_VERSION}" \
--constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
+ fi; \
+ fi; \
+ if [[ ${INSTALL_FROM_DOCKER_CONTEXT_FILES} == "true" ]]; then \
+ reinstalling_apache_airflow_packages=$(ls /docker-context-files/apache?airflow*.{whl,tar.gz} 2>/dev/null || true); \
+ # We want to install apache airflow packages with constraints \
+ if [[ "${reinstalling_apache_airflow_packages}" != "" ]]; then \
+ if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then \
+ pip install --force-reinstall --upgrade --upgrade-strategy eager \
+ --user ${reinstalling_apache_airflow_packages}; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
+ else \
+ pip install --force-reinstall --upgrade --upgrade-strategy only-if-needed \
+ --user ${reinstalling_apache_airflow_packages} --constraint "${AIRFLOW_CONSTRAINTS_LOCATION}"; \
+ pip install --upgrade "pip==${AIRFLOW_PIP_VERSION}"; \
+ fi; \
+ fi ; \
+ # All the others we want to reinstall as-is, without dependencies \
+ reinstalling_other_packages=$(ls /docker-context-files/*.{whl,tar.gz} 2>/dev/null | \
Review comment:
But for all other packages, we want to just install whatever we have there without dependencies. If the packages are result of `pip download`, this serves the purpose of 'offline installation" mechanism where you download all packages and store them in artifact registry and 'vet' and use them for installation.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #13422: Removes pip download when installing from local packages
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13422:
URL: https://github.com/apache/airflow/pull/13422#issuecomment-753279646
I will need to merge this one before making #13409 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org