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 9a0080c20b Improve handling of entry and exit to common Breeze
commands (#23395)
9a0080c20b is described below
commit 9a0080c20bb2c4a9c0f6ccf1ece79bde895688ac
Author: Jarek Potiuk <[email protected]>
AuthorDate: Mon May 2 12:56:46 2022 +0200
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
---
.../commands/configuration_and_maintenance.py | 2 +-
.../airflow_breeze/commands/developer_commands.py | 2 +
dev/breeze/src/airflow_breeze/shell/enter_shell.py | 77 +++++-----------------
.../airflow_breeze/utils/docker_command_utils.py | 2 +-
.../utils/rebuild_image_if_needed.py | 58 ++++++++++++++++
5 files changed, 79 insertions(+), 62 deletions(-)
diff --git
a/dev/breeze/src/airflow_breeze/commands/configuration_and_maintenance.py
b/dev/breeze/src/airflow_breeze/commands/configuration_and_maintenance.py
index a83d24c921..47811c3660 100644
--- a/dev/breeze/src/airflow_breeze/commands/configuration_and_maintenance.py
+++ b/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.")
diff --git a/dev/breeze/src/airflow_breeze/commands/developer_commands.py
b/dev/breeze/src/airflow_breeze/commands/developer_commands.py
index b665ee1685..3338a32cb6 100644
--- a/dev/breeze/src/airflow_breeze/commands/developer_commands.py
+++ b/dev/breeze/src/airflow_breeze/commands/developer_commands.py
@@ -60,6 +60,7 @@ from airflow_breeze.utils.docker_command_utils import (
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,
diff --git a/dev/breeze/src/airflow_breeze/shell/enter_shell.py
b/dev/breeze/src/airflow_breeze/shell/enter_shell.py
index 3f6ff5b1b2..951d36220a 100644
--- a/dev/breeze/src/airflow_breeze/shell/enter_shell.py
+++ b/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']
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 579e74aa18..c7c54630fa 100644
--- a/dev/breeze/src/airflow_breeze/utils/docker_command_utils.py
+++ b/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.
diff --git a/dev/breeze/src/airflow_breeze/utils/rebuild_image_if_needed.py
b/dev/breeze/src/airflow_breeze/utils/rebuild_image_if_needed.py
new file mode 100644
index 0000000000..a3981fa542
--- /dev/null
+++ b/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)
