[airflow] branch main updated: Fix coertion for VERBOSE and DRY_RUN env variables (#27366)

[po...@apache.org]

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 8ea77a5980 Fix coertion for VERBOSE and DRY_RUN env variables (#27366)
8ea77a5980 is described below

commit 8ea77a5980dea0ffe0ad5928db6bcdff636aadce
Author: Jarek Potiuk <ja...@potiuk.com>
AuthorDate: Sat Oct 29 20:16:29 2022 +0200

    Fix coertion for VERBOSE and DRY_RUN env variables (#27366)
    
    The #27191 change introduced globally shareable flags for verbose
    and dry-run flags in Breeze however it also changed behaviour of
    the "false" string passed by VERBOSE/DRY_RUN env variables and
    crashed breeze when one of those variables was set to empty string.
    
    As a result CI's "VERBOSE" "false" flag in static checks was
    treated as "truthy" and static checks output contained a lot of
    noise from non-failing static checks.
    
    It turns out that "convert" method of click can also take "strings"
    as parameter because coertion of env variables happens inside the
    convert method (which mkes sense if you think about it) so the
    convert method should accept both string and target type values,
    This is explained in convert method docs but I missed it :(.
    
    This change fixes both problems by introducing explicit coertion
    of the variables from all kinds of string values - both when the
    flags are parsed but also before (sometimes we need verbose flag
    from variable (when we run breeze in scripts) even before the
    flag is actually parsed by click so additionally to coercing the
    flag, we also pre-emptively run the same coertion when importing
    the "shared_options" module.
    
    Also just for the sake of it, we accept all kinds of truth-y,
    falsy strings now - true/false/yes/no/t/f/y/n in all kinds of
    casing and shortcuts.
---
 dev/breeze/src/airflow_breeze/utils/coertions.py   | 27 ++++++++++++++++++++++
 .../src/airflow_breeze/utils/custom_param_types.py |  5 ++--
 .../src/airflow_breeze/utils/shared_options.py     | 12 ++++++++--
 3 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/dev/breeze/src/airflow_breeze/utils/coertions.py b/dev/breeze/src/airflow_breeze/utils/coertions.py
new file mode 100644
index 0000000000..015c363faf
--- /dev/null
+++ b/dev/breeze/src/airflow_breeze/utils/coertions.py
@@ -0,0 +1,27 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations
+
+
+def coerce_bool_value(value: str | bool) -> bool:
+    if isinstance(value, bool):
+        return value
+    elif not value:  # handle "" and other false-y coerce-able values
+        return False
+    else:
+        return value[0].lower() in ["t", "y"]  # handle all kinds of truth-y/yes-y/false-y/non-sy strings
diff --git a/dev/breeze/src/airflow_breeze/utils/custom_param_types.py b/dev/breeze/src/airflow_breeze/utils/custom_param_types.py
index 2c55e77790..79838919bd 100644
--- a/dev/breeze/src/airflow_breeze/utils/custom_param_types.py
+++ b/dev/breeze/src/airflow_breeze/utils/custom_param_types.py
@@ -29,6 +29,7 @@ from airflow_breeze.utils.cache import (
     read_from_cache_file,
     write_to_cache_file,
 )
+from airflow_breeze.utils.coertions import coerce_bool_value
 from airflow_breeze.utils.console import get_console
 from airflow_breeze.utils.recording import generating_command_images
 from airflow_breeze.utils.shared_options import set_dry_run, set_forced_answer, set_verbose
@@ -112,7 +113,7 @@ class VerboseOption(ParamType):
     """
 
     def convert(self, value, param, ctx):
-        set_verbose(value)
+        set_verbose(coerce_bool_value(value))
         return super().convert(value, param, ctx)
 
 
@@ -122,7 +123,7 @@ class DryRunOption(ParamType):
     """
 
     def convert(self, value, param, ctx):
-        set_dry_run(value)
+        set_dry_run(coerce_bool_value(value))
         return super().convert(value, param, ctx)
 
 
diff --git a/dev/breeze/src/airflow_breeze/utils/shared_options.py b/dev/breeze/src/airflow_breeze/utils/shared_options.py
index 10d9e875b8..6e9b1e5b54 100644
--- a/dev/breeze/src/airflow_breeze/utils/shared_options.py
+++ b/dev/breeze/src/airflow_breeze/utils/shared_options.py
@@ -19,7 +19,15 @@ from __future__ import annotations
 
 import os
 
-__verbose_value: bool = os.environ.get("VERBOSE", "false")[0].lower() == "t"
+from airflow_breeze.utils.coertions import coerce_bool_value
+
+
+def __get_default_bool_value(env_var: str) -> bool:
+    string_val = os.environ.get(env_var, "")
+    return coerce_bool_value(string_val)
+
+
+__verbose_value: bool = __get_default_bool_value("VERBOSE")
 
 
 def set_verbose(verbose: bool):
@@ -33,7 +41,7 @@ def get_verbose(verbose_override: bool | None = None) -> bool:
     return verbose_override
 
 
-__dry_run_value: bool = os.environ.get("DRY_RUN", "false")[0].lower() == "t"
+__dry_run_value: bool = __get_default_bool_value("DRY_RUN")
 
 
 def set_dry_run(dry_run: bool):