Improve handling of entry and exit to common Breeze commands (#23395)

This PR improves handling of both entry and exit to common
Breeze commands:

* at entry all common commands check if rebuild of image is needed
* when you exit and there is an error from shell commands, rather
  than printing stack trace an error message is printed

dev/breeze/src/airflow_breeze/commands/configuration_and_maintenance.py

@@ -405,7 +405,7 @@ def free_space(verbose: bool, dry_run: bool, answer: str):
 @option_dry_run
 def resource_check(verbose: bool, dry_run: bool):
     shell_params = ShellParams(verbose=verbose, python=DEFAULT_PYTHON_MAJOR_MINOR_VERSION)
-    check_docker_resources(verbose, shell_params.airflow_image_name, dry_run=dry_run)
+    check_docker_resources(shell_params.airflow_image_name, verbose=verbose, dry_run=dry_run)
 
 
 @main.command(name="fix-ownership", help="Fix ownership of source files to be same as host user.")

dev/breeze/src/airflow_breeze/commands/developer_commands.py

@@ -60,6 +60,7 @@
     get_extra_docker_flags,
 )
 from airflow_breeze.utils.path_utils import AIRFLOW_SOURCES_ROOT
+from airflow_breeze.utils.rebuild_image_if_needed import rebuild_ci_image_if_needed
 from airflow_breeze.utils.run_utils import assert_pre_commit_installed, run_command
 
 DEVELOPER_COMMANDS = {
@@ -348,6 +349,7 @@ def build_docs(
 ):
     """Build documentation in the container."""
     params = BuildCiParams(github_repository=github_repository, python=DEFAULT_PYTHON_MAJOR_MINOR_VERSION)
+    rebuild_ci_image_if_needed(build_params=params, dry_run=dry_run, verbose=verbose)
     ci_image_name = params.airflow_image_name
     doc_builder = DocBuildParams(
         package_filter=package_filter,

dev/breeze/src/airflow_breeze/shell/enter_shell.py

@@ -17,57 +17,21 @@
 """Command to enter container shell for Breeze."""
 import subprocess
 import sys
-from pathlib import Path
-from typing import Dict, Optional, Union
+from typing import Optional, Union
 
-from airflow_breeze import global_constants
-from airflow_breeze.build_image.ci.build_ci_image import build_ci_image
-from airflow_breeze.build_image.ci.build_ci_params import BuildCiParams
 from airflow_breeze.shell.shell_params import ShellParams
-from airflow_breeze.utils.cache import (
-    read_and_validate_value_from_cache,
-    read_from_cache_file,
-    write_to_cache_file,
-)
+from airflow_breeze.utils.cache import read_from_cache_file
 from airflow_breeze.utils.console import get_console
 from airflow_breeze.utils.docker_command_utils import (
-    SOURCE_OF_DEFAULT_VALUES_FOR_VARIABLES,
-    VARIABLES_IN_CACHE,
-    check_docker_compose_version,
     check_docker_is_running,
     check_docker_resources,
-    check_docker_version,
     construct_env_variables_docker_compose_command,
 )
-from airflow_breeze.utils.path_utils import BUILD_CACHE_DIR
+from airflow_breeze.utils.rebuild_image_if_needed import rebuild_ci_image_if_needed
 from airflow_breeze.utils.run_utils import filter_out_none, run_command
 from airflow_breeze.utils.visuals import ASCIIART, ASCIIART_STYLE, CHEATSHEET, CHEATSHEET_STYLE
 
 
-def synchronize_cached_params(parameters_passed_by_the_user: Dict[str, str]) -> Dict[str, str]:
-    """
-    Synchronizes cached params with arguments passed via dictionary.
-
-    It will read from cache parameters that are missing and writes back in case user
-    actually provided new values for those parameters. It synchronizes all cacheable parameters.
-
-    :param parameters_passed_by_the_user: user args passed
-    :return: updated args
-    """
-    updated_params = dict(parameters_passed_by_the_user)
-    for param in VARIABLES_IN_CACHE:
-        if param in parameters_passed_by_the_user:
-            param_name = VARIABLES_IN_CACHE[param]
-            user_param_value = parameters_passed_by_the_user[param]
-            if user_param_value is not None:
-                write_to_cache_file(param_name, user_param_value)
-            else:
-                param_value = getattr(global_constants, SOURCE_OF_DEFAULT_VALUES_FOR_VARIABLES[param])
-                _, user_param_value = read_and_validate_value_from_cache(param_name, param_value)
-            updated_params[param] = user_param_value
-    return updated_params
-
-
 def enter_shell(**kwargs) -> Union[subprocess.CompletedProcess, subprocess.CalledProcessError]:
     """
     Executes entering shell using the parameters passed as kwargs:
@@ -88,14 +52,11 @@ def enter_shell(**kwargs) -> Union[subprocess.CompletedProcess, subprocess.Calle
             '[warning]Please make sure Docker is installed and running.[/]'
         )
         sys.exit(1)
-    check_docker_version(verbose)
-    check_docker_compose_version(verbose)
-    updated_kwargs = synchronize_cached_params(kwargs)
     if read_from_cache_file('suppress_asciiart') is None:
         get_console().print(ASCIIART, style=ASCIIART_STYLE)
     if read_from_cache_file('suppress_cheatsheet') is None:
         get_console().print(CHEATSHEET, style=CHEATSHEET_STYLE)
-    enter_shell_params = ShellParams(**filter_out_none(**updated_kwargs))
+    enter_shell_params = ShellParams(**filter_out_none(**kwargs))
     return run_shell_with_build_image_checks(verbose, dry_run, enter_shell_params)
 
 
@@ -116,28 +77,24 @@ def run_shell_with_build_image_checks(
     :param dry_run: do not execute "write" commands - just print what would happen
     :param shell_params: parameters of the execution
     """
-    check_docker_resources(verbose, shell_params.airflow_image_name, dry_run=dry_run)
-    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)
-    if build_ci_image_check_cache.exists():
-        if verbose:
-            get_console().print(f'[info]{shell_params.the_image_type} image already built locally.[/]')
-    else:
-        get_console().print(
-            f'[warning]{shell_params.the_image_type} image not built locally. Forcing build.[/]'
-        )
-        ci_image_params.force_build = True
-
-    build_ci_image(verbose, dry_run=dry_run, with_ci_group=False, ci_image_params=ci_image_params)
+    rebuild_ci_image_if_needed(build_params=shell_params, dry_run=dry_run, verbose=verbose)
     shell_params.print_badge_info()
     cmd = ['docker-compose', 'run', '--service-ports', "-e", "BREEZE", '--rm', 'airflow']
     cmd_added = shell_params.command_passed
     env_variables = construct_env_variables_docker_compose_command(shell_params)
     if cmd_added is not None:
         cmd.extend(['-c', cmd_added])
-    return run_command(cmd, verbose=verbose, dry_run=dry_run, env=env_variables, text=True)
+
+    command_result = run_command(
+        cmd, verbose=verbose, dry_run=dry_run, env=env_variables, text=True, check=False
+    )
+    if command_result.returncode == 0:
+        return command_result
+    else:
+        get_console().print(f"[red]Error {command_result.returncode} returned[/]")
+        if verbose:
+            get_console().print(command_result.stderr)
+        return command_result
 
 
 def stop_exec_on_error(returncode: int):
@@ -147,7 +104,7 @@ def stop_exec_on_error(returncode: int):
 
 def find_airflow_container(verbose, dry_run) -> Optional[str]:
     exec_shell_params = ShellParams(verbose=verbose, dry_run=dry_run)
-    check_docker_resources(verbose, exec_shell_params.airflow_image_name, dry_run)
+    check_docker_resources(exec_shell_params.airflow_image_name, verbose=verbose, dry_run=dry_run)
     exec_shell_params.print_badge_info()
     env_variables = construct_env_variables_docker_compose_command(exec_shell_params)
     cmd = ['docker-compose', 'ps', '--all', '--filter', 'status=running', 'airflow']

dev/breeze/src/airflow_breeze/utils/docker_command_utils.py

@@ -111,7 +111,7 @@ def get_extra_docker_flags(mount_sources: str) -> List[str]:
 
 
 def check_docker_resources(
-    verbose: bool, airflow_image_name: str, dry_run: bool
+    airflow_image_name: str, verbose: bool, dry_run: bool
 ) -> Union[subprocess.CompletedProcess, subprocess.CalledProcessError]:
     """
     Check if we have enough resources to run docker. This is done via running script embedded in our image.

dev/breeze/src/airflow_breeze/utils/rebuild_image_if_needed.py

@@ -0,0 +1,58 @@
+# 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 pathlib import Path
+from typing import Union
+
+from airflow_breeze.build_image.ci.build_ci_image import build_ci_image
+from airflow_breeze.build_image.ci.build_ci_params import BuildCiParams
+from airflow_breeze.shell.shell_params import ShellParams
+from airflow_breeze.utils.console import get_console
+from airflow_breeze.utils.docker_command_utils import (
+    check_docker_compose_version,
+    check_docker_resources,
+    check_docker_version,
+)
+from airflow_breeze.utils.path_utils import BUILD_CACHE_DIR
+
+
+def rebuild_ci_image_if_needed(
+    build_params: Union[ShellParams, BuildCiParams], dry_run: bool, verbose: bool
+) -> None:
+    """
+    Rebuilds CI image if needed and user confirms it.
+
+    :param build_params: parameters of the shell
+    :param dry_run: whether it's a dry_run
+    :param verbose: should we print verbose messages
+    """
+    check_docker_version(verbose=verbose)
+    check_docker_compose_version(verbose=verbose)
+    check_docker_resources(build_params.airflow_image_name, verbose=verbose, dry_run=dry_run)
+    build_ci_image_check_cache = Path(
+        BUILD_CACHE_DIR, build_params.airflow_branch, f".built_{build_params.python}"
+    )
+    ci_image_params = BuildCiParams(python=build_params.python, upgrade_to_newer_dependencies=False)
+    if build_ci_image_check_cache.exists():
+        if verbose:
+            get_console().print(f'[info]{build_params.the_image_type} image already built locally.[/]')
+    else:
+        get_console().print(
+            f'[warning]{build_params.the_image_type} image not built locally. Forcing build.[/]'
+        )
+        ci_image_params.force_build = True
+    build_ci_image(verbose, dry_run=dry_run, with_ci_group=False, ci_image_params=ci_image_params)