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/02/05 16:54:26 UTC
[GitHub] [airflow] potiuk opened a new pull request #14107: Restores flexible installation version, fixes manual tag build process.
potiuk opened a new pull request #14107:
URL: https://github.com/apache/airflow/pull/14107
Revert "Fix Commands to install Airflow in docker/install_airflow.sh (#14099)"
This reverts commit 68758b826076e93fadecf599108a4d304dd87ac7.
<!--
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] potiuk commented on a change in pull request #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571114793
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,8 +113,8 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
- export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
We might want to change it later (but again we got into `master build` vs PR case.
----------------------------------------------------------------
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 #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#issuecomment-774223676
Just did. Actually I think master is broken now before this one gets merged https://github.com/potiuk/airflow/actions/runs/541317863
----------------------------------------------------------------
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 #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571138631
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,8 +113,8 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
- export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
> I think what I am failing to understand is if `AIRFLOW_INSTALL_VERSION` needs `==VERSION` or `<=VERSION` etc. How does
>
> https://github.com/apache/airflow/blob/fc67521f31a0c9a74dadda8d5f0ac32c07be218d/.github/workflows/ci.yml#L489
>
> work as it looks like `"--build-arg" "AIRFLOW_INSTALL_VERSION=${INSTALL_AIRFLOW_VERSION}"` it is passed verbatim
Yes because that only affects the way how PROD image is built. For CI image it is different - we always built it from sources, but then when we enter the CI Image we can remove/reinstall airflow using either a PyPI version or locally built wheel/sdist. This allows to use the same CI image to test different airflow version (this is how we are testing backport providers - we take the CI image, reinstall airflow to 1.10.14 from pypi and then install all backport providers built locally.
That's what `INSTALL_AIRFLOW_VERSION: "1.10.14"` line in ci.yml is. Similarly we take our providers and install them in 2.0.0 version as there was a change in get_provider_info() schema and we want to make sure that the providers install in 2.0.0 version as well. This is is what `INSTALL_AIRFLOW_VERSION: "2.0.0"` does in ci.yml.
Also we use it to install airflow from wheels or sdist packages provided `INSTALL_AIRFLOW_VERSION: "${{ matrix.package-format }}"` -> in this case we are simply using `wheel` or `sdist` there.
Here is the specification from breeze explaining it:
```
-a, --install-airflow-version INSTALL_AIRFLOW_VERSION
If specified, installs Airflow directly from PIP released version. This happens at
image building time in production image and at container entering time for CI image. One of:
2.0.0 1.10.14 1.10.12 1.10.11 1.10.10 1.10.9 none wheel sdist
When 'none' is used, you can install airflow from local packages. When building image,
airflow package should be added to 'docker-context-files' and
--install-from-docker-context-files flag should be used. When running an image, airflow
package should be added to dist folder and --install-packages-from-dist flag should be used.
```
If you look at entrypoint_ci.sh this piece of code is exected in entrypoint_ci.sh:
```
if [[ -z ${INSTALL_AIRFLOW_VERSION=} ]]; then
export PYTHONPATH=${AIRFLOW_SOURCES}
echo
echo "Using already installed airflow version"
echo
"${AIRFLOW_SOURCES}/airflow/www/ask_for_recompile_assets_if_needed.sh"
# Cleanup the logs, tmp when entering the environment
sudo rm -rf "${AIRFLOW_SOURCES}"/logs/*
sudo rm -rf "${AIRFLOW_SOURCES}"/tmp/*
mkdir -p "${AIRFLOW_SOURCES}"/logs/
mkdir -p "${AIRFLOW_SOURCES}"/tmp/
elif [[ ${INSTALL_AIRFLOW_VERSION} == "none" ]]; then
echo
echo "Skip installing airflow - only install wheel/tar.gz packages that are present locally"
echo
uninstall_airflow_and_providers
elif [[ ${INSTALL_AIRFLOW_VERSION} == "wheel" ]]; then
echo
echo "Install airflow from wheel package with [${AIRFLOW_EXTRAS}] extras but uninstalling providers."
echo
uninstall_airflow_and_providers
install_airflow_from_wheel "[${AIRFLOW_EXTRAS}]"
uninstall_providers
elif [[ ${INSTALL_AIRFLOW_VERSION} == "sdist" ]]; then
echo
echo "Install airflow from sdist package with [${AIRFLOW_EXTRAS}] extras but uninstalling providers."
echo
uninstall_airflow_and_providers
install_airflow_from_sdist "[${AIRFLOW_EXTRAS}]"
uninstall_providers
else
echo
echo "Install airflow from PyPI including [${AIRFLOW_EXTRAS}] extras"
echo
install_released_airflow_version "${INSTALL_AIRFLOW_VERSION}" "[${AIRFLOW_EXTRAS}]"
fi
```
We might want to change that varialle name in CI image to avoid confusion - maybe REINSTALL_AIRFLOW_IN_CI ? - then we could make two parameters in breeze instead of the shared one:
* `--install-airflow-version 2.0.0 1.10.14 1.10.12 1.10.11 1.10.10 1.10.9` for PROD image
* `--reinstall-airflow-in-ci 2.0.0 1.10.14 1.10.12 1.10.11 1.10.10 1.10.9 none wheel sdist` for CI image
WDYT ?
----------------------------------------------------------------
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] ashb merged pull request #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
ashb merged pull request #14107:
URL: https://github.com/apache/airflow/pull/14107
----------------------------------------------------------------
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 #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571139040
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,8 +113,8 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
- export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
(But I'd do it in separate PR).
----------------------------------------------------------------
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 #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571110843
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,7 +113,7 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
Yep. Good catch. Fixed.
----------------------------------------------------------------
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] kaxil commented on a change in pull request #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571111649
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,8 +113,8 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
- export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
What about this?
Also the one in https://github.com/apache/airflow/blob/master/docs/apache-airflow/production-deployment.rst is `AIRFLOW_INSTALL_VERSION` and the one here is `INSTALL_AIRFLOW_VERSION`
----------------------------------------------------------------
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] kaxil commented on pull request #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
kaxil commented on pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#issuecomment-774213778
@potiuk Can you also run this one on your fork -- so we can merge it when it is 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 pull request #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#issuecomment-774183338
See my comment https://github.com/apache/airflow/pull/14107#discussion_r571138631 - I think this version I proposed now is fine, and I can make a follow-up PR where I separate `INSTALL_AIRFLOW_VERSION` in prod from `REINSTALL_AIRFLOW_IN_CI` to avoid any confusion.
----------------------------------------------------------------
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 #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#issuecomment-774230104
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] kaxil commented on a change in pull request #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571116420
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,8 +113,8 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
- export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
I think what I am failing to understand is if `AIRFLOW_INSTALL_VERSION` needs `==VERSION` or `<=VERSION` etc. How does https://github.com/apache/airflow/blob/fc67521f31a0c9a74dadda8d5f0ac32c07be218d/.github/workflows/ci.yml#L489 work as it looks like `"--build-arg" "AIRFLOW_INSTALL_VERSION=${INSTALL_AIRFLOW_VERSION}"` it is passed verbatim
----------------------------------------------------------------
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 #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571147783
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,8 +113,8 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
- export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
(side comment) The main reason why `INSTALL_AIRFLOW_VERSION` is used in both PROD and CI is that we already had this and It was close enough in both CI and PROD to reuse the same variable/breeze flaag. But yeah - they behave differently and I think confusion that introduces is a a bit too much.
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,8 +113,8 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
- export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
(side comment) The main reason why `INSTALL_AIRFLOW_VERSION` is used in both PROD and CI is that we already had this and It was close enough in both CI and PROD to reuse the same variable/breeze flag. But yeah - they behave differently and I think confusion that introduces is a a bit too much.
----------------------------------------------------------------
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] ashb commented on pull request #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#issuecomment-774280782
`ERROR: Invalid requirement: '.[devel_ci]=='` will be fixed by this right?
----------------------------------------------------------------
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 #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571115950
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,8 +113,8 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
- export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
We should likely rename it ti "AIRLFOW_INSTALL_SPECIFICATION" to avoid confusion. I will make a PR for that.
----------------------------------------------------------------
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] kaxil commented on a change in pull request #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571111649
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,8 +113,8 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
- export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
What about this?
Also the one in https://github.com/apache/airflow/blob/master/docs/apache-airflow/production-deployment.rst is AIRFLOW_INSTALL_VERSION and the one here is INSTALL_AIRFLOW_VERSION
----------------------------------------------------------------
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] kaxil commented on a change in pull request #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571113635
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,8 +113,8 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
- export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
How does this work:
https://github.com/apache/airflow/blob/fc67521f31a0c9a74dadda8d5f0ac32c07be218d/.github/workflows/ci.yml#L394
----------------------------------------------------------------
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 #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#issuecomment-774168824
I think this version fixes it all now:
* INSTALL_AIRFLOW_VERSION => version to install
* AIRFLOW_VERSION_SPECIFICATION => "==2.0.2rc1" or "<2.1"
When I set AIRFLOW_VERSION_SPECIFICATION in breeze I set to to "==INSTALL_AIRFLOW_VERSION".
----------------------------------------------------------------
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 #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571115950
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,8 +113,8 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
- export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
We should likely rename it ti "AIRFLOW_INSTALL_SPECIFICATION" (or similar) to avoid confusion. I will make a PR for that.
----------------------------------------------------------------
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] kaxil commented on a change in pull request #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571112288
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,8 +113,8 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
- export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
Just trying to understand how is it passed
----------------------------------------------------------------
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 #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571117065
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,8 +113,8 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
- export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
Glad that we have more pairs of eyes looking.
----------------------------------------------------------------
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 #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571114255
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,8 +113,8 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
- export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
Yeah. It is a mix of unified attempt of naming which has not been completed:
```
"--build-arg" "AIRFLOW_INSTALL_VERSION=${INSTALL_AIRFLOW_VERSION}"
```
From _build_images.sh
----------------------------------------------------------------
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] kaxil commented on a change in pull request #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571108941
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,7 +113,7 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
The line below now fails
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,7 +113,7 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
```
export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
```
----------------------------------------------------------------
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] kaxil commented on a change in pull request #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571109865
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,7 +113,7 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
Also the one in https://github.com/apache/airflow/blob/master/docs/apache-airflow/production-deployment.rst is `AIRFLOW_INSTALL_VERSION` and the one here is `INSTALL_AIRFLOW_VERSION`
----------------------------------------------------------------
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] kaxil commented on a change in pull request #14107: Restores flexible installation version, fixes manual tag build process.
Posted by GitBox <gi...@apache.org>.
kaxil commented on a change in pull request #14107:
URL: https://github.com/apache/airflow/pull/14107#discussion_r571113921
##########
File path: scripts/ci/images/ci_build_dockerhub.sh
##########
@@ -113,8 +113,8 @@ else
export DOCKER_CACHE="local"
# Name the image based on the TAG rather than based on the branch name
export FORCE_AIRFLOW_PROD_BASE_TAG="${DOCKER_TAG}"
- export INSTALL_AIRFLOW_VERSION="${DOCKER_TAG%-python*}"
- export AIRFLOW_CONSTRAINTS_REFERENCE="constraints-${INSTALL_AIRFLOW_VERSION}"
+ export INSTALL_AIRFLOW_VERSION="==${DOCKER_TAG%-python*}"
Review comment:
https://github.com/apache/airflow/blob/fc67521f31a0c9a74dadda8d5f0ac32c07be218d/.github/workflows/ci.yml#L489
----------------------------------------------------------------
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