Skip to content

Commit

Permalink
Rename build arg for upgrade-to-newer-dependencies (apache#38637)
Browse files Browse the repository at this point in the history
The "UPGRADE_TO_NEWER_DEPENDENCIES" build arg was a bit misleading
as it was not a true/false value but rather "random hash"/"false".

This PR makes it a bit more explicit:

* the arg is named UPGRADE_INVALIDATION_STRING now
* it's default is "" not false and all conditions are set to check
  for != or == "".

The inputs/outputs in CI job remain as they were - "true/false" but
this PR makes it clearer.
  • Loading branch information
potiuk authored Apr 2, 2024
1 parent ba41e5a commit 1934c8b
Show file tree
Hide file tree
Showing 10 changed files with 78 additions and 81 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/prod-image-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ on: # yamllint disable-line rule:truthy
required: true
type: string
upgrade-to-newer-dependencies:
description: "Whether to attempt to upgrade image to newer dependencies (false/RANDOM_VALUE)"
description: "Whether to attempt to upgrade image to newer dependencies (true/false)"
required: true
type: string
chicken-egg-providers:
Expand Down
11 changes: 5 additions & 6 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -903,7 +903,7 @@ function install_airflow() {
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/postgres,}
echo "${COLOR_YELLOW}Postgres client installation is disabled. Extra 'postgres' installations were therefore omitted.${COLOR_RESET}"
fi
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then
if [[ "${UPGRADE_INVALIDATION_STRING=}" != "" ]]; then
echo
echo "${COLOR_BLUE}Remove airflow and all provider packages installed before potentially${COLOR_RESET}"
echo
Expand Down Expand Up @@ -961,13 +961,12 @@ COPY <<"EOF" /install_additional_dependencies.sh
#!/usr/bin/env bash
set -euo pipefail

: "${UPGRADE_TO_NEWER_DEPENDENCIES:?Should be true or false}"
: "${ADDITIONAL_PYTHON_DEPS:?Should be set}"

. "$( dirname "${BASH_SOURCE[0]}" )/common.sh"

function install_additional_dependencies() {
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then
if [[ "${UPGRADE_INVALIDATION_STRING=}" != "" ]]; then
echo
echo "${COLOR_BLUE}Installing additional dependencies while upgrading to newer dependencies${COLOR_RESET}"
echo
Expand Down Expand Up @@ -1489,7 +1488,7 @@ ARG AIRFLOW_VERSION_SPECIFICATION
# set to "." and "/opt/airflow" respectively.
ARG AIRFLOW_INSTALLATION_METHOD="apache-airflow"
# By default we do not upgrade to latest dependencies
ARG UPGRADE_TO_NEWER_DEPENDENCIES="false"
ARG UPGRADE_INVALIDATION_STRING=""
ARG AIRFLOW_SOURCES_FROM
ARG AIRFLOW_SOURCES_TO

Expand Down Expand Up @@ -1533,7 +1532,7 @@ ENV AIRFLOW_PIP_VERSION=${AIRFLOW_PIP_VERSION} \
AIRFLOW_USER_HOME_DIR=${AIRFLOW_USER_HOME_DIR} \
AIRFLOW_HOME=${AIRFLOW_HOME} \
AIRFLOW_UID=${AIRFLOW_UID} \
UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES}
UPGRADE_INVALIDATION_STRING=${UPGRADE_INVALIDATION_STRING}


# Copy all scripts required for installation - changing any of those should lead to
Expand Down Expand Up @@ -1573,7 +1572,7 @@ RUN bash /scripts/docker/install_packaging_tools.sh; \
bash /scripts/docker/create_prod_venv.sh; \
if [[ ${AIRFLOW_PRE_CACHED_PIP_PACKAGES} == "true" && \
${INSTALL_PACKAGES_FROM_CONTEXT} == "false" && \
${UPGRADE_TO_NEWER_DEPENDENCIES} == "false" ]]; then \
${UPGRADE_INVALIDATION_STRING} == "" ]]; then \
bash /scripts/docker/install_airflow_dependencies_from_branch_tip.sh; \
fi

Expand Down
9 changes: 4 additions & 5 deletions Dockerfile.ci
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ function install_airflow() {
AIRFLOW_EXTRAS=${AIRFLOW_EXTRAS/postgres,}
echo "${COLOR_YELLOW}Postgres client installation is disabled. Extra 'postgres' installations were therefore omitted.${COLOR_RESET}"
fi
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then
if [[ "${UPGRADE_INVALIDATION_STRING=}" != "" ]]; then
echo
echo "${COLOR_BLUE}Remove airflow and all provider packages installed before potentially${COLOR_RESET}"
echo
Expand Down Expand Up @@ -764,13 +764,12 @@ COPY <<"EOF" /install_additional_dependencies.sh
#!/usr/bin/env bash
set -euo pipefail

: "${UPGRADE_TO_NEWER_DEPENDENCIES:?Should be true or false}"
: "${ADDITIONAL_PYTHON_DEPS:?Should be set}"

. "$( dirname "${BASH_SOURCE[0]}" )/common.sh"

function install_additional_dependencies() {
if [[ "${UPGRADE_TO_NEWER_DEPENDENCIES}" != "false" ]]; then
if [[ "${UPGRADE_INVALIDATION_STRING=}" != "" ]]; then
echo
echo "${COLOR_BLUE}Installing additional dependencies while upgrading to newer dependencies${COLOR_RESET}"
echo
Expand Down Expand Up @@ -1324,11 +1323,11 @@ COPY --from=scripts install_airflow.sh /scripts/docker/
# dependency resolution and we do not need to limit the versions of the dependencies
#
ARG EAGER_UPGRADE_ADDITIONAL_REQUIREMENTS=""
ARG UPGRADE_TO_NEWER_DEPENDENCIES="false"
ARG UPGRADE_INVALIDATION_STRING=""
ARG VERSION_SUFFIX_FOR_PYPI=""

ENV EAGER_UPGRADE_ADDITIONAL_REQUIREMENTS=${EAGER_UPGRADE_ADDITIONAL_REQUIREMENTS} \
UPGRADE_TO_NEWER_DEPENDENCIES=${UPGRADE_TO_NEWER_DEPENDENCIES} \
UPGRADE_INVALIDATION_STRING=${UPGRADE_INVALIDATION_STRING} \
VERSION_SUFFIX_FOR_PYPI=${VERSION_SUFFIX_FOR_PYPI}

# The goal of this line is to install the dependencies from the most current pyproject.toml from sources
Expand Down
2 changes: 1 addition & 1 deletion dev/breeze/doc/ci/02_images.md
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@ can be used for CI images:
| `AIRFLOW_CONSTRAINTS_LOCATION` | | If not empty, it will override the source of the constraints with the specified URL or file. |
| `AIRFLOW_CONSTRAINTS_REFERENCE` | | reference (branch or tag) from GitHub repository from which constraints are used. By default it is set to `constraints-main` but can be `constraints-2-X`. |
| `AIRFLOW_EXTRAS` | `all` | extras to install |
| `UPGRADE_TO_NEWER_DEPENDENCIES` | `false` | If set to a value different than "false" the dependencies are upgraded to newer versions. In CI it is set to build id. |
| `UPGRADE_INVALIDATION_STRING` | | If set to any random value the dependencies are upgraded to newer versions. In CI it is set to build id. |
| `AIRFLOW_PRE_CACHED_PIP_PACKAGES` | `true` | Allows to pre-cache airflow PIP packages from the GitHub of Apache Airflow This allows to optimize iterations for Image builds and speeds up CI jobs. |
| `ADDITIONAL_AIRFLOW_EXTRAS` | | additional extras to install |
| `ADDITIONAL_PYTHON_DEPS` | | additional Python dependencies to install |
Expand Down
2 changes: 1 addition & 1 deletion dev/breeze/doc/ci/04_selective_checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ am overview of possible labels and their meaning:
| all versions | all-versions, *-versions-* | Run tests for all python and k8s versions. |
| full tests needed | full-tests-needed | Run complete set of tests (might be with default or all python/k8s versions) |
| non committer build | is-committer-build | If set then even for non-committer builds, the scripts used for images are used from target branch. |
| upgrade to newer dependencies | upgrade-to-newer-dependencies | If set then dependencies in the CI image build are upgraded to the newer ones. |
| upgrade to newer dependencies | upgrade-to-newer-dependencies | If set to true (default false) then dependencies in the CI image build are upgraded to the newer ones. |
| use public runners | runs-on-as-json-default | Force using public runners even for Committer runs. |


Expand Down
2 changes: 1 addition & 1 deletion dev/breeze/src/airflow_breeze/params/build_ci_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def prepare_arguments_for_docker_build_command(self) -> list[str]:
self._req_arg("CONSTRAINTS_GITHUB_REPOSITORY", self.constraints_github_repository)
self._req_arg("PYTHON_BASE_IMAGE", self.python_base_image)
if self.upgrade_to_newer_dependencies:
self._opt_arg("UPGRADE_TO_NEWER_DEPENDENCIES", f"{random.randrange(2**32):x}")
self._opt_arg("UPGRADE_INVALIDATION_STRING", f"{random.randrange(2**32):x}")
if self.eager_upgrade_additional_requirements:
# in case eager upgrade additional requirements have EOL, connect them together
self._opt_arg(
Expand Down
Loading

0 comments on commit 1934c8b

Please sign in to comment.