You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by po...@apache.org on 2022/04/28 19:12:41 UTC

[airflow] branch main updated: Remove confusion about upgrade-to-newer-dependencies breeze param (#23334)

This is an automated email from the ASF dual-hosted git repository.

potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git


The following commit(s) were added to refs/heads/main by this push:
     new b6db0e90ae Remove confusion about upgrade-to-newer-dependencies breeze param (#23334)
b6db0e90ae is described below

commit b6db0e90aeb30133086716a433cab9dca7408a54
Author: Jarek Potiuk <ja...@polidea.com>
AuthorDate: Thu Apr 28 21:12:35 2022 +0200

    Remove confusion about upgrade-to-newer-dependencies breeze param (#23334)
    
    The "upgrade-to-newer-dependencies" in the image can take "false"
    "true" and <RANDOM> value (the RANDOM value is used to always
    invalidate docker cache).
    
    This has been carried to Breeze command line, but this was a source
    of major confusion as the name of the parameter suggest bool value.
    
    The change modifies the parameter to be flag, and when the flag
    is set the parameter is set to random during image building.
    
    That should help with recent bug when image was always rebuilt
    without checking if it should be rebuilt.
---
 .../src/airflow_breeze/build_image/ci/build_ci_params.py |  2 +-
 .../airflow_breeze/build_image/prod/build_prod_params.py |  2 +-
 dev/breeze/src/airflow_breeze/commands/common_options.py |  5 ++---
 .../src/airflow_breeze/commands/release_management.py    |  9 ++++-----
 dev/breeze/src/airflow_breeze/shell/enter_shell.py       |  2 +-
 .../src/airflow_breeze/utils/docker_command_utils.py     | 16 +++++++++++++---
 images/breeze/output-build-image.svg                     |  5 ++---
 images/breeze/output-build-prod-image.svg                |  5 ++---
 scripts/ci/selective_ci_checks.sh                        | 14 ++++----------
 9 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/dev/breeze/src/airflow_breeze/build_image/ci/build_ci_params.py b/dev/breeze/src/airflow_breeze/build_image/ci/build_ci_params.py
index da5364d087..af37c71dbe 100644
--- a/dev/breeze/src/airflow_breeze/build_image/ci/build_ci_params.py
+++ b/dev/breeze/src/airflow_breeze/build_image/ci/build_ci_params.py
@@ -33,7 +33,7 @@ class BuildCiParams:
     CI build parameters. Those parameters are used to determine command issued to build CI image.
     """
 
-    upgrade_to_newer_dependencies: str = "false"
+    upgrade_to_newer_dependencies: bool = False
     python: str = "3.7"
     airflow_branch: str = AIRFLOW_BRANCH
     build_id: int = 0
diff --git a/dev/breeze/src/airflow_breeze/build_image/prod/build_prod_params.py b/dev/breeze/src/airflow_breeze/build_image/prod/build_prod_params.py
index 116fae9333..91a06e6e3f 100644
--- a/dev/breeze/src/airflow_breeze/build_image/prod/build_prod_params.py
+++ b/dev/breeze/src/airflow_breeze/build_image/prod/build_prod_params.py
@@ -52,7 +52,7 @@ class BuildProdParams:
     empty_image: bool = False
     airflow_is_in_context: bool = False
     install_packages_from_context: bool = False
-    upgrade_to_newer_dependencies: str = "false"
+    upgrade_to_newer_dependencies: bool = False
     airflow_version: str = get_airflow_version()
     python: str = "3.7"
     airflow_branch_for_pypi_preloading: str = AIRFLOW_BRANCH
diff --git a/dev/breeze/src/airflow_breeze/commands/common_options.py b/dev/breeze/src/airflow_breeze/commands/common_options.py
index e169697cd2..a2d2e8c0f9 100644
--- a/dev/breeze/src/airflow_breeze/commands/common_options.py
+++ b/dev/breeze/src/airflow_breeze/commands/common_options.py
@@ -197,9 +197,8 @@ option_debian_version = click.option(
 option_upgrade_to_newer_dependencies = click.option(
     "-u",
     '--upgrade-to-newer-dependencies',
-    default="false",
-    show_default=True,
-    help='When other than "false", upgrade all PIP packages to latest.',
+    is_flag=True,
+    help='When set, upgrade all PIP packages to latest.',
     envvar='UPGRADE_TO_NEWER_DEPENDENCIES',
 )
 option_additional_extras = click.option(
diff --git a/dev/breeze/src/airflow_breeze/commands/release_management.py b/dev/breeze/src/airflow_breeze/commands/release_management.py
index 72e2756878..33765f9afd 100644
--- a/dev/breeze/src/airflow_breeze/commands/release_management.py
+++ b/dev/breeze/src/airflow_breeze/commands/release_management.py
@@ -320,13 +320,12 @@ def generate_constraints(
     set_forced_answer(answer)
     if run_in_parallel:
         given_answer = user_confirm(
-            f"Did you build all CI images {python_versions} with --upgrade-to-newer-dependencies "
-            f"true flag set?",
+            f"Did you build all CI images {python_versions} with --upgrade-to-newer-dependencies flag set?",
             timeout=None,
         )
     else:
         given_answer = user_confirm(
-            f"Did you build CI image {python} with --upgrade-to-newer-dependencies true flag set?",
+            f"Did you build CI image {python} with --upgrade-to-newer-dependencies flag set?",
             timeout=None,
         )
     if given_answer != Answer.YES:
@@ -334,7 +333,7 @@ def generate_constraints(
             get_console().print("\n[info]Use this command to build the images:[/]\n")
             get_console().print(
                 f"     breeze build-image --run-in-parallel --python-versions '{python_versions}' "
-                f"--upgrade-to-newer-dependencies true\n"
+                f"--upgrade-to-newer-dependencies\n"
             )
         else:
             shell_params = ShellParams(
@@ -343,7 +342,7 @@ def generate_constraints(
             get_console().print("\n[info]Use this command to build the image:[/]\n")
             get_console().print(
                 f"     breeze build-image --python'{shell_params.python}' "
-                f"--upgrade-to-newer-dependencies true\n"
+                f"--upgrade-to-newer-dependencies\n"
             )
         sys.exit(1)
     if run_in_parallel:
diff --git a/dev/breeze/src/airflow_breeze/shell/enter_shell.py b/dev/breeze/src/airflow_breeze/shell/enter_shell.py
index 51294d55bb..c9d93abb3d 100644
--- a/dev/breeze/src/airflow_breeze/shell/enter_shell.py
+++ b/dev/breeze/src/airflow_breeze/shell/enter_shell.py
@@ -120,7 +120,7 @@ def run_shell_with_build_image_checks(
     build_ci_image_check_cache = Path(
         BUILD_CACHE_DIR, shell_params.airflow_branch, f".built_{shell_params.python}"
     )
-    ci_image_params = BuildCiParams(python=shell_params.python, upgrade_to_newer_dependencies="false")
+    ci_image_params = BuildCiParams(python=shell_params.python, upgrade_to_newer_dependencies=False)
     if build_ci_image_check_cache.exists():
         get_console().print(f'[info]{shell_params.the_image_type} image already built locally.[/]')
     else:
diff --git a/dev/breeze/src/airflow_breeze/utils/docker_command_utils.py b/dev/breeze/src/airflow_breeze/utils/docker_command_utils.py
index 4598a6ce97..813ad67ea2 100644
--- a/dev/breeze/src/airflow_breeze/utils/docker_command_utils.py
+++ b/dev/breeze/src/airflow_breeze/utils/docker_command_utils.py
@@ -18,6 +18,7 @@
 import os
 import re
 import subprocess
+from random import randint
 from typing import Dict, List, Tuple, Union
 
 from airflow_breeze.build_image.ci.build_ci_params import BuildCiParams
@@ -286,7 +287,14 @@ def construct_arguments_for_docker_build_command(
     image_params: Union[BuildCiParams, BuildProdParams], required_args: List[str], optional_args: List[str]
 ) -> List[str]:
     """
-    Constructs docker compose command arguments list based on parameters passed
+    Constructs docker compose command arguments list based on parameters passed. Maps arguments to
+    argument values.
+
+    It maps:
+    * all the truthy/falsy values are converted to "true" / "false" respectively
+    * if upgrade_to_newer_dependencies is set to True, it is replaced by a random string to account
+      for the need of always triggering upgrade for docker build.
+
     :param image_params: parameters of the image
     :param required_args: build argument that are required
     :param optional_args: build arguments that are optional (should not be used if missing or empty)
@@ -295,8 +303,10 @@ def construct_arguments_for_docker_build_command(
 
     def get_env_variable_value(arg_name: str):
         value = str(getattr(image_params, arg_name))
-        value = "true" if value == "True" else value
-        value = "false" if value == "False" else value
+        value = "true" if value.lower() in ["true", "t", "yes", "y"] else value
+        value = "false" if value.lower() in ["false", "f", "no", "n"] else value
+        if arg_name == "upgrade_to_newer_dependencies" and value == "true":
+            value = f"{randint(0, 2**32):x}"
         return value
 
     args_command = []
diff --git a/images/breeze/output-build-image.svg b/images/breeze/output-build-image.svg
index 01b392f174..f55442611c 100644
--- a/images/breeze/output-build-image.svg
+++ b/images/breeze/output-build-image.svg
@@ -1,4 +1,4 @@
-<svg width="1720.0" height="1528" viewBox="0 0 1720.0 1528"
+<svg width="1720.0" height="1506" viewBox="0 0 1720.0 1506"
      xmlns="http://www.w3.org/2000/svg">
     <style>
         @font-face {
@@ -124,8 +124,7 @@
 <div><span class="r4">│</span><span class="r1">  </span><span class="r5">--python</span><span class="r1">                         </span><span class="r6">-p</span><span class="r1">  </span><span class="r1">Python major/minor version used in Airflow image for images.                 </span><span class="r1">  </span><span class="r4">│</span></div>
 <div><span class="r4">│</span><span class="r1">                                       </span><span class="r7">(&gt;3.7&lt; | 3.8 | 3.9 | 3.10)                                  </span><span class="r1">                 </span><span class="r1">  </span><span class="r4">│</span></div>
 <div><span class="r4">│</span><span class="r1">                                       </span><span class="r4">[default: 3.7]                                              </span><span class="r1">                 </span><span class="r1">  </span><span class="r4">│</span></div>
-<div><span class="r4">│</span><span class="r1">  </span><span class="r5">--upgrade-to-newer-dependencies</span><span class="r1">  </span><span class="r6">-u</span><span class="r1">  </span><span class="r1">When other than &quot;false&quot;, upgrade all PIP packages to latest.</span><span class="r1"> </span><span class="r7">(TEXT)</span><span class="r1">          </span><span class="r1">  </span><span class="r4">│</span></div>
-<div><span class="r4">│</span><span class="r1">                                       </span><span class="r4">[default: false]                                            </span><span class="r1"> </span><span class="r1">                </span><span class="r1">  </span><span class="r4">│</span></div>
+<div><span class="r4">│</span><span class="r1">  </span><span class="r5">--upgrade-to-newer-dependencies</span><span class="r1">  </span><span class="r6">-u</span><span class="r1">  </span><span class="r1">When set, upgrade all PIP packages to latest.                                </span><span class="r1">  </span><span class="r4">│</span></div>
 <div><span class="r4">│</span><span class="r1">  </span><span class="r5">--debian-version</span><span class="r1">                 </span><span class="r6">-d</span><span class="r1">  </span><span class="r1">Debian version used for the image.</span><span class="r1"> </span><span class="r7">(bullseye | buster)</span><span class="r1"> </span><span class="r4">[default: bullseye]</span><span class="r1">   </span><span class="r1">  </span><span class="r4">│</span></div>
 <div><span class="r4">│</span><span class="r1">  </span><span class="r5">--image-tag</span><span class="r1">                      </span><span class="r6">-t</span><span class="r1">  </span><span class="r1">Tag added to the default naming conventions of Airflow CI/PROD images.</span><span class="r1"> </span><span class="r7">(TEXT)</span><span class="r1">  </span><span class="r4">│</span></div>
 <div><span class="r4">│</span><span class="r1">  </span><span class="r5">--docker-cache</span><span class="r1">                   </span><span class="r6">-c</span><span class="r1">  </span><span class="r1">Cache option for image used during the build.</span><span class="r1"> </span><span class="r7">(pulled | local | disabled)</span><span class="r1">    </span><span class="r1">  </span><span class="r4">│</span></div>
diff --git a/images/breeze/output-build-prod-image.svg b/images/breeze/output-build-prod-image.svg
index def3afbc88..7d693c4d73 100644
--- a/images/breeze/output-build-prod-image.svg
+++ b/images/breeze/output-build-prod-image.svg
@@ -1,4 +1,4 @@
-<svg width="1720.0" height="1902" viewBox="0 0 1720.0 1902"
+<svg width="1720.0" height="1880" viewBox="0 0 1720.0 1880"
      xmlns="http://www.w3.org/2000/svg">
     <style>
         @font-face {
@@ -125,8 +125,7 @@
 <div><span class="r4">│</span><span class="r1">                                       </span><span class="r7">(&gt;3.7&lt; | 3.8 | 3.9 | 3.10)                                  </span><span class="r1">                 </span><span class="r1">  </span><span class="r4">│</span></div>
 <div><span class="r4">│</span><span class="r1">                                       </span><span class="r4">[default: 3.7]                                              </span><span class="r1">                 </span><span class="r1">  </span><span class="r4">│</span></div>
 <div><span class="r4">│</span><span class="r1">  </span><span class="r5">--install-airflow-version</span><span class="r1">        </span><span class="r6">-V</span><span class="r1">  </span><span class="r1">Install version of Airflow from PyPI.</span><span class="r1"> </span><span class="r7">(TEXT)</span><span class="r1">                                 </span><span class="r1">  </span><span class="r4">│</span></div>
-<div><span class="r4">│</span><span class="r1">  </span><span class="r5">--upgrade-to-newer-dependencies</span><span class="r1">  </span><span class="r6">-u</span><span class="r1">  </span><span class="r1">When other than &quot;false&quot;, upgrade all PIP packages to latest.</span><span class="r1"> </span><span class="r7">(TEXT)</span><span class="r1">          </span><span class="r1">  </span><span class="r4">│</span></div>
-<div><span class="r4">│</span><span class="r1">                                       </span><span class="r4">[default: false]                                            </span><span class="r1"> </span><span class="r1">                </span><span class="r1">  </span><span class="r4">│</span></div>
+<div><span class="r4">│</span><span class="r1">  </span><span class="r5">--upgrade-to-newer-dependencies</span><span class="r1">  </span><span class="r6">-u</span><span class="r1">  </span><span class="r1">When set, upgrade all PIP packages to latest.                                </span><span class="r1">  </span><span class="r4">│</span></div>
 <div><span class="r4">│</span><span class="r1">  </span><span class="r5">--debian-version</span><span class="r1">                 </span><span class="r6">-d</span><span class="r1">  </span><span class="r1">Debian version used for the image.</span><span class="r1"> </span><span class="r7">(bullseye | buster)</span><span class="r1"> </span><span class="r4">[default: bullseye]</span><span class="r1">   </span><span class="r1">  </span><span class="r4">│</span></div>
 <div><span class="r4">│</span><span class="r1">  </span><span class="r5">--image-tag</span><span class="r1">                      </span><span class="r6">-t</span><span class="r1">  </span><span class="r1">Tag added to the default naming conventions of Airflow CI/PROD images.</span><span class="r1"> </span><span class="r7">(TEXT)</span><span class="r1">  </span><span class="r4">│</span></div>
 <div><span class="r4">│</span><span class="r1">  </span><span class="r5">--docker-cache</span><span class="r1">                   </span><span class="r6">-c</span><span class="r1">  </span><span class="r1">Cache option for image used during the build.</span><span class="r1"> </span><span class="r7">(pulled | local | disabled)</span><span class="r1">    </span><span class="r1">  </span><span class="r4">│</span></div>
diff --git a/scripts/ci/selective_ci_checks.sh b/scripts/ci/selective_ci_checks.sh
index 85f7a1c07e..c9f215c53c 100755
--- a/scripts/ci/selective_ci_checks.sh
+++ b/scripts/ci/selective_ci_checks.sh
@@ -44,13 +44,8 @@ fi
 
 function check_upgrade_to_newer_dependencies_needed() {
     if [[ ${GITHUB_EVENT_NAME=} == 'push' || ${GITHUB_EVENT_NAME=} == "scheduled" ]]; then
-        # Trigger upgrading to latest constraints when we are in push or schedule event. We use the
-        # random string so that it always triggers rebuilding layer in the docker image
-        # Each build that upgrades to latest constraints will get truly latest constraints, not those
-        # cached in the image because docker layer will get invalidated.
-        # This upgrade_to_newer_dependencies variable can later be overridden
-        # in case we find that any of the setup.* files changed (see below)
-        upgrade_to_newer_dependencies="${RANDOM}"
+        # Trigger upgrading to latest constraints when we are in push or schedule event
+        upgrade_to_newer_dependencies="true"
     fi
 }
 
@@ -358,9 +353,8 @@ function check_if_setup_files_changed() {
 
     if [[ $(count_changed_files) != "0" ]]; then
         # In case the setup files changed, we automatically force upgrading to newer dependencies
-        # no matter what was set before. We set it to random number to make sure that it will be
-        # always invalidating the layer in Docker that triggers installing the dependencies
-        upgrade_to_newer_dependencies="${RANDOM}"
+        # no matter what was set before.
+        upgrade_to_newer_dependencies="true"
     fi
     start_end::group_end
 }