This is an automated email from the ASF dual-hosted git repository. potiuk pushed a commit to branch v2-3-test in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 43589dd887574f13a14de88e37e66b9b601dd359 Author: Jarek Potiuk <[email protected]> AuthorDate: Tue May 3 22:22:42 2022 +0200 Unify approach for user questions asked in Breeze (#23335) This change documents and unifies the approach we've taken for the user inut handling when it comes to confirmation questions. (cherry picked from commit ebc1f14db3a1b14f2535462e97a6407f48b19f7c) --- breeze | 14 ++- .../doc/adr/0012-asking-user-for-confirmation.md | 99 ++++++++++++++++++++++ .../build_image/ci/build_ci_image.py | 12 ++- .../src/airflow_breeze/commands/ci_image_tools.py | 2 - .../src/airflow_breeze/commands/common_options.py | 9 +- .../commands/configuration_and_maintenance.py | 21 +++-- .../airflow_breeze/commands/custom_param_types.py | 13 +++ .../airflow_breeze/commands/developer_commands.py | 3 - .../commands/production_image_tools.py | 2 - .../airflow_breeze/commands/release_management.py | 6 +- dev/breeze/src/airflow_breeze/utils/confirm.py | 42 ++++++--- .../src/airflow_breeze/utils/md5_build_check.py | 3 +- dev/breeze/src/airflow_breeze/utils/path_utils.py | 13 +++ dev/breeze/src/airflow_breeze/utils/reinstall.py | 11 ++- dev/breeze/src/airflow_breeze/utils/run_utils.py | 4 +- 15 files changed, 205 insertions(+), 49 deletions(-) diff --git a/breeze b/breeze index b433fac7f9..d84f4bb8f0 100755 --- a/breeze +++ b/breeze @@ -21,19 +21,22 @@ MY_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" BREEZE_BINARY=breeze COLOR_RED=$'\e[31m' +COLOR_BLUE=$'\e[34m' COLOR_RESET=$'\e[0m' COLOR_YELLOW=$'\e[33m' function manual_instructions() { - echo "Please run those commands manually (you might need to restart shell between them):" + echo + echo "${COLOR_BLUE}Please run those commands manually (you might need to restart shell between them):i${COLOR_RESET}" echo echo " pip -m install pipx" echo " pipx ensurepath" echo " pipx install -e '${MY_DIR}/dev/breeze/'" echo " breeze setup-autocomplete --force" echo - echo " After that, both pipx and breeze should be available ono your path" - exit + echo " After that, both pipx and breeze should be available on your path" + echo + exit 1 } function check_breeze_installed() { @@ -61,7 +64,10 @@ function check_breeze_installed() { if [[ ${breeze_on_path} != 0 ]]; then echo "${COLOR_RED}The '${BREEZE_BINARY}' is not on path. Breeze should be installed and 'breeze' should be available on your PATH!${COLOR_RESET}" export TIMEOUT=0 - if "${MY_DIR}/scripts/tools/confirm" "Installing breeze?"; then + echo + echo "${COLOR_YELLOW}Installing Breeze. This will install breeze via pipx and modify your local rc file for ${SHELL}${COLOR_RESET}" + echo + if "${MY_DIR}/scripts/tools/confirm" "installing and modifying the startup scripts"; then pipx ensurepath --force pipx install -e "${MY_DIR}/dev/breeze/" --force ${BREEZE_BINARY} setup-autocomplete --force --answer yes diff --git a/dev/breeze/doc/adr/0012-asking-user-for-confirmation.md b/dev/breeze/doc/adr/0012-asking-user-for-confirmation.md new file mode 100644 index 0000000000..6b63780b23 --- /dev/null +++ b/dev/breeze/doc/adr/0012-asking-user-for-confirmation.md @@ -0,0 +1,99 @@ +<!-- + 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. + --> + +<!-- START doctoc generated TOC please keep comment here to allow auto update --> +<!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE --> +**Table of Contents** *generated with [DocToc](https://github.com/thlorenz/doctoc)* + +- [12. Asking user for confirmation](#12-asking-user-for-confirmation) + - [Status](#status) + - [Context](#context) + - [Decision](#decision) + - [Consequences](#consequences) + +<!-- END doctoc generated TOC please keep comment here to allow auto update --> + +# 12. Asking user for confirmation + +Date: 2022-04-28 + +## Status + +Accepted + +## Context + +When we run a number of breeze commands, we sometimes ask the user for confirmation. This happens in a few +cases: + +1. When the user attempts to run a potentially disruptive operation (cleanup, delete, modifying startup + scripts) +2. When the user attempts to run a potentially long operation or operation that needs fast internet access +3. When an action generates an auxiliary action that we determined we need to run in order to make sure + we use the latest version of the environment (like upgrading breeze dependencies or pulling and building + latest image). + +In case 1) - we should present the user with information on what is going to change and the user should +not be surprised with the effect of it. + +In case 2) - we should not break the workflow of the user, unless this is really necessary or the user +chooses to break his workflow. This is important in situation when the user needs to perform an action +quickly and disrupting the workflow would be annoying and undesired. In this case the confirmation of +the "long" action should not be "necessary" - when user does not expect the action to wait for confirmation. + +In case 3) - we want to gently nudge the user to run an action when we guess it is advised. We should not +break the regular workflows, but we want the user to "eventually" run the upgrade to use the same +environment as others (and avoiding the problem where thing "work for me but not the others". + +Breeze is also used for unattended actions - and then any questions would prevent the actions from running, +so it should be possible to disable the questions and assume an answer in those cases. + +## Decision + +We take the following approach with the confirmation: + +1) If there is a potentially disruptive command - we always ask the user for confirmation and explicit +confirmation is necessary. Default action in this case is "quit" so if the user just presses enter +the operation should quit without making any change. + +2) If there is a long-running (but not disruptive) operation that user directly requested (for example +building and image by `build-image` command), there should be no question asked unless there is some +condition that could change the length of the action (for example when we realise that the user have not +rebased to the latest main and rebuilding the image might take far longer than initially anticipated by +the user). In case of such unexpected condition the default answer should be "quit" as well without timeout. + +3) If there is an auxiliary actions (for example building image) that would prevent (or delay) another action +of the user - the default action should be "no" (no upgrade) and there should be a short timeout (few seconds) +so that the original action can be executed without too much waiting. +In all cases an upgrade needs an explicit 'y' answer by the user. + +Pressing "ENTER" when asked triggers the default action if it is specified. + +User can "force" any answer by adding `--answer <ANSWER>` command line switch to any command line that +might ask a question - and it will force `<ANSWER>` as an immediate action to all questions. + +Default action (if used) is capitalised in allowed answers. Timeout is displayed when present. + +## Consequences + +As a consequence, the user will not accidentally make any disruptive changes without confirming it. The user +will not have a broken workflow even if upgrade is recommended - they will just get a short annoying delay and +question asked whether to upgrade, but the upgrade will not run without the user's consent. The short +delay will introduce a gentle "nagging" of the user to upgrade their breeze or image which will lead to +"eventually consistent" versions of both for the users. diff --git a/dev/breeze/src/airflow_breeze/build_image/ci/build_ci_image.py b/dev/breeze/src/airflow_breeze/build_image/ci/build_ci_image.py index d2c12a23bf..3b09d61602 100644 --- a/dev/breeze/src/airflow_breeze/build_image/ci/build_ci_image.py +++ b/dev/breeze/src/airflow_breeze/build_image/ci/build_ci_image.py @@ -27,7 +27,7 @@ from airflow_breeze.build_image.ci.build_ci_params import ( ) from airflow_breeze.utils.cache import touch_cache_file from airflow_breeze.utils.ci_group import ci_group -from airflow_breeze.utils.confirm import Answer, user_confirm +from airflow_breeze.utils.confirm import STANDARD_TIMEOUT, Answer, user_confirm from airflow_breeze.utils.console import get_console from airflow_breeze.utils.docker_command_utils import ( construct_docker_build_command, @@ -69,16 +69,20 @@ def should_we_run_the_build(build_ci_params: BuildCiParams, verbose: bool) -> bo ): return False try: - answer = user_confirm(message="Do you want to build image?", timeout=5, default_answer=Answer.NO) + answer = user_confirm( + message="Do you want to build the image?", timeout=STANDARD_TIMEOUT, default_answer=Answer.NO + ) if answer == answer.YES: if is_repo_rebased(build_ci_params.github_repository, build_ci_params.airflow_branch): return True else: get_console().print( - "\n[warning]This might take a lot of time, w" "e think you should rebase first.[/]\n" + "\n[warning]This might take a lot of time, we think you should rebase first.[/]\n" ) answer = user_confirm( - "But if you really, really want - you can do it", timeout=5, default_answer=Answer.NO + "But if you really, really want - you can do it. Are you really sure?", + timeout=STANDARD_TIMEOUT, + default_answer=Answer.NO, ) if answer == Answer.YES: return True diff --git a/dev/breeze/src/airflow_breeze/commands/ci_image_tools.py b/dev/breeze/src/airflow_breeze/commands/ci_image_tools.py index 15fd2d52f5..90761417ea 100644 --- a/dev/breeze/src/airflow_breeze/commands/ci_image_tools.py +++ b/dev/breeze/src/airflow_breeze/commands/ci_image_tools.py @@ -65,7 +65,6 @@ from airflow_breeze.commands.common_options import ( ) from airflow_breeze.commands.main import main from airflow_breeze.utils.ci_group import ci_group -from airflow_breeze.utils.confirm import set_forced_answer from airflow_breeze.utils.console import get_console from airflow_breeze.utils.pulll_image import run_pull_image, run_pull_in_parallel from airflow_breeze.utils.python_versions import get_python_version_list @@ -219,7 +218,6 @@ def build_image( get_console().print(f"[error]Error when building image! {info}") sys.exit(return_code) - set_forced_answer(answer) parameters_passed = filter_out_none(**kwargs) if build_multiple_images: with ci_group(f"Building images sequentially {python_versions}", enabled=with_ci_group): diff --git a/dev/breeze/src/airflow_breeze/commands/common_options.py b/dev/breeze/src/airflow_breeze/commands/common_options.py index 2b0831e44a..7193743fad 100644 --- a/dev/breeze/src/airflow_breeze/commands/common_options.py +++ b/dev/breeze/src/airflow_breeze/commands/common_options.py @@ -19,7 +19,12 @@ import multiprocessing as mp import click -from airflow_breeze.commands.custom_param_types import BetterChoice, CacheableChoice, CacheableDefault +from airflow_breeze.commands.custom_param_types import ( + AnswerChoice, + BetterChoice, + CacheableChoice, + CacheableDefault, +) from airflow_breeze.global_constants import ( ALLOWED_BACKENDS, ALLOWED_BUILD_CACHE, @@ -50,7 +55,7 @@ option_dry_run = click.option( option_answer = click.option( "-a", "--answer", - type=BetterChoice(['y', 'n', 'q', 'yes', 'no', 'quit']), + type=AnswerChoice(choices=['y', 'n', 'q', 'yes', 'no', 'quit']), help="Force answer to questions.", envvar='ANSWER', ) 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 47811c3660..5f4d12c83b 100644 --- a/dev/breeze/src/airflow_breeze/commands/configuration_and_maintenance.py +++ b/dev/breeze/src/airflow_breeze/commands/configuration_and_maintenance.py @@ -40,7 +40,7 @@ from airflow_breeze.commands.main import main from airflow_breeze.global_constants import DEFAULT_PYTHON_MAJOR_MINOR_VERSION, MOUNT_ALL from airflow_breeze.shell.shell_params import ShellParams from airflow_breeze.utils.cache import check_if_cache_exists, delete_cache, touch_cache_file -from airflow_breeze.utils.confirm import Answer, set_forced_answer, user_confirm +from airflow_breeze.utils.confirm import STANDARD_TIMEOUT, Answer, user_confirm from airflow_breeze.utils.console import get_console from airflow_breeze.utils.docker_command_utils import ( check_docker_resources, @@ -130,7 +130,6 @@ CONFIGURATION_AND_MAINTENANCE_PARAMETERS = { @option_dry_run @option_github_repository def cleanup(verbose: bool, dry_run: bool, github_repository: str, all: bool, answer: Optional[str]): - set_forced_answer(answer) if all: get_console().print( "\n[info]Removing cache of parameters, clean up docker cache " @@ -162,7 +161,7 @@ def cleanup(verbose: bool, dry_run: bool, github_repository: str, all: bool, ans '--force', ] docker_rmi_command_to_execute.extend(images) - given_answer = user_confirm("Are you sure?", timeout=None) + given_answer = user_confirm("Are you sure with the removal?") if given_answer == Answer.YES: run_command(docker_rmi_command_to_execute, verbose=verbose, dry_run=dry_run, check=False) elif given_answer == Answer.QUIT: @@ -170,14 +169,14 @@ def cleanup(verbose: bool, dry_run: bool, github_repository: str, all: bool, ans else: get_console().print("[light_blue]No locally downloaded images to remove[/]\n") get_console().print("Pruning docker images") - given_answer = user_confirm("Are you sure?", timeout=None) + given_answer = user_confirm("Are you sure with the removal?") if given_answer == Answer.YES: system_prune_command_to_execute = ['docker', 'system', 'prune'] run_command(system_prune_command_to_execute, verbose=verbose, dry_run=dry_run, check=False) elif given_answer == Answer.QUIT: sys.exit(0) get_console().print(f"Removing build cache dir ${BUILD_CACHE_DIR}") - given_answer = user_confirm("Are you sure?", timeout=None) + given_answer = user_confirm("Are you sure with the removal?") if given_answer == Answer.YES: if not dry_run: shutil.rmtree(BUILD_CACHE_DIR, ignore_errors=True) @@ -215,7 +214,7 @@ def self_upgrade(force: bool, use_current_airflow_sources: bool): if force: reinstall_breeze(breeze_sources) else: - ask_to_reinstall_breeze(breeze_sources) + ask_to_reinstall_breeze(breeze_sources, timeout=None) else: warn_non_editable() sys.exit(1) @@ -235,7 +234,6 @@ def setup_autocomplete(verbose: bool, dry_run: bool, force: bool, answer: Option """ Enables autocompletion of breeze commands. """ - set_forced_answer(answer) # Determine if the shell is bash/zsh/powershell. It helps to build the autocomplete path detected_shell = os.environ.get('SHELL') detected_shell = None if detected_shell is None else detected_shell.split(os.sep)[-1] @@ -247,8 +245,10 @@ def setup_autocomplete(verbose: bool, dry_run: bool, force: bool, answer: Option AIRFLOW_SOURCES_ROOT / "dev" / "breeze" / "autocomplete" / f"{NAME}-complete-{detected_shell}.sh" ) get_console().print(f"[info]Activation command script is available here: {autocomplete_path}[/]\n") - get_console().print(f"[info]We need to add above script to your {detected_shell} profile.[/]\n") - given_answer = user_confirm("Should we proceed ?", default_answer=Answer.NO, timeout=3) + get_console().print(f"[warning]We need to add above script to your {detected_shell} profile.[/]\n") + given_answer = user_confirm( + "Should we proceed with modifying the script?", default_answer=Answer.NO, timeout=STANDARD_TIMEOUT + ) if given_answer == Answer.YES: if detected_shell == 'bash': script_path = str(Path('~').expanduser() / '.bash_completion') @@ -388,8 +388,7 @@ def change_config( @option_dry_run @option_answer def free_space(verbose: bool, dry_run: bool, answer: str): - set_forced_answer(answer) - if user_confirm("Are you sure to run free-space and perform cleanup?", timeout=None) == Answer.YES: + if user_confirm("Are you sure to run free-space and perform cleanup?") == Answer.YES: run_command(["sudo", "swapoff", "-a"], verbose=verbose, dry_run=dry_run) run_command(["sudo", "rm", "-f", "/swapfile"], verbose=verbose, dry_run=dry_run) run_command(["sudo", "apt-get", "clean"], verbose=verbose, dry_run=dry_run, check=False) diff --git a/dev/breeze/src/airflow_breeze/commands/custom_param_types.py b/dev/breeze/src/airflow_breeze/commands/custom_param_types.py index 4737aa2737..45c4ee0ff8 100644 --- a/dev/breeze/src/airflow_breeze/commands/custom_param_types.py +++ b/dev/breeze/src/airflow_breeze/commands/custom_param_types.py @@ -26,6 +26,7 @@ from airflow_breeze.utils.cache import ( read_from_cache_file, write_to_cache_file, ) +from airflow_breeze.utils.confirm import set_forced_answer from airflow_breeze.utils.console import get_console from airflow_breeze.utils.recording import output_file_for_recording @@ -53,6 +54,18 @@ class BetterChoice(click.Choice): return f"[{choices_str}]" +class AnswerChoice(BetterChoice): + """ + Stores forced answer if it has been selected + """ + + name = "AnswerChoice" + + def convert(self, value, param, ctx): + set_forced_answer(value) + return super().convert(value, param, ctx) + + @dataclass class CacheableDefault: value: Any diff --git a/dev/breeze/src/airflow_breeze/commands/developer_commands.py b/dev/breeze/src/airflow_breeze/commands/developer_commands.py index 3338a32cb6..598190d5d9 100644 --- a/dev/breeze/src/airflow_breeze/commands/developer_commands.py +++ b/dev/breeze/src/airflow_breeze/commands/developer_commands.py @@ -53,7 +53,6 @@ from airflow_breeze.global_constants import ( from airflow_breeze.pre_commit_ids import PRE_COMMIT_LIST from airflow_breeze.shell.enter_shell import enter_shell, find_airflow_container from airflow_breeze.shell.shell_params import ShellParams -from airflow_breeze.utils.confirm import set_forced_answer from airflow_breeze.utils.console import get_console from airflow_breeze.utils.docker_command_utils import ( construct_env_variables_docker_compose_command, @@ -230,7 +229,6 @@ def shell( extra_args: Tuple, ): """Enter breeze.py environment. this is the default command use when no other is selected.""" - set_forced_answer(answer) if verbose or dry_run: get_console().print("\n[success]Welcome to breeze.py[/]\n") get_console().print(f"\n[success]Root of Airflow Sources = {AIRFLOW_SOURCES_ROOT}[/]\n") @@ -295,7 +293,6 @@ def start_airflow( extra_args: Tuple, ): """Enter breeze.py environment and starts all Airflow components in the tmux session.""" - set_forced_answer(answer) enter_shell( verbose=verbose, dry_run=dry_run, diff --git a/dev/breeze/src/airflow_breeze/commands/production_image_tools.py b/dev/breeze/src/airflow_breeze/commands/production_image_tools.py index 52fd3ec2a3..fcd09549cd 100644 --- a/dev/breeze/src/airflow_breeze/commands/production_image_tools.py +++ b/dev/breeze/src/airflow_breeze/commands/production_image_tools.py @@ -66,7 +66,6 @@ from airflow_breeze.commands.custom_param_types import BetterChoice from airflow_breeze.commands.main import main from airflow_breeze.global_constants import ALLOWED_INSTALLATION_METHODS, DEFAULT_EXTRAS from airflow_breeze.utils.ci_group import ci_group -from airflow_breeze.utils.confirm import set_forced_answer from airflow_breeze.utils.console import get_console from airflow_breeze.utils.pulll_image import run_pull_image, run_pull_in_parallel from airflow_breeze.utils.python_versions import get_python_version_list @@ -271,7 +270,6 @@ def build_prod_image( get_console().print(f"[error]Error when building image! {info}") sys.exit(return_code) - set_forced_answer(answer) parameters_passed = filter_out_none(**kwargs) if build_multiple_images: with ci_group(f"Building images sequentially {python_versions}", enabled=with_ci_group): diff --git a/dev/breeze/src/airflow_breeze/commands/release_management.py b/dev/breeze/src/airflow_breeze/commands/release_management.py index 33765f9afd..4655b6197c 100644 --- a/dev/breeze/src/airflow_breeze/commands/release_management.py +++ b/dev/breeze/src/airflow_breeze/commands/release_management.py @@ -48,7 +48,7 @@ from airflow_breeze.global_constants import ( ) from airflow_breeze.shell.shell_params import ShellParams from airflow_breeze.utils.ci_group import ci_group -from airflow_breeze.utils.confirm import Answer, set_forced_answer, user_confirm +from airflow_breeze.utils.confirm import Answer, user_confirm from airflow_breeze.utils.console import get_console from airflow_breeze.utils.constraints import run_generate_constraints, run_generate_constraints_in_parallel from airflow_breeze.utils.docker_command_utils import ( @@ -192,7 +192,6 @@ def prepare_provider_documentation( skip_package_verification: bool, packages: List[str], ): - set_forced_answer(answer) shell_params = ShellParams( verbose=verbose, mount_sources=MOUNT_ALL, @@ -317,16 +316,13 @@ def generate_constraints( generate_constraints_mode: str, with_ci_group: bool, ): - 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 flag set?", - timeout=None, ) else: given_answer = user_confirm( f"Did you build CI image {python} with --upgrade-to-newer-dependencies flag set?", - timeout=None, ) if given_answer != Answer.YES: if run_in_parallel: diff --git a/dev/breeze/src/airflow_breeze/utils/confirm.py b/dev/breeze/src/airflow_breeze/utils/confirm.py index 45a9026edb..8cfb1c69fd 100644 --- a/dev/breeze/src/airflow_breeze/utils/confirm.py +++ b/dev/breeze/src/airflow_breeze/utils/confirm.py @@ -19,11 +19,13 @@ import sys from enum import Enum from typing import Optional +STANDARD_TIMEOUT = 10 + class Answer(Enum): - YES = (0,) - NO = (1,) - QUIT = 2 + YES = "y" + NO = "n" + QUIT = "q" forced_answer: Optional[str] = None @@ -36,7 +38,7 @@ def set_forced_answer(answer: Optional[str]): def user_confirm( message: str, - timeout: Optional[float], + timeout: Optional[float] = None, default_answer: Optional[Answer] = Answer.NO, quit_allowed: bool = True, ) -> Answer: @@ -46,8 +48,7 @@ def user_confirm( :rtype: object :param message: message to display to the user (should end with the question mark) :param timeout: time given user to answer - :param default_answer: default value returned on timeout. If no default - the question is - repeated until user answers. + :param default_answer: default value returned on timeout. If no default - is set, the timeout is ignored. :param quit_allowed: whether quit answer is allowed :return: """ @@ -61,11 +62,31 @@ def user_confirm( user_status = force print(f"Forced answer for '{message}': {force}") else: + if default_answer: + # Capitalise default answer + allowed_answers = allowed_answers.replace( + default_answer.value, default_answer.value.upper() + ) + timeout_answer = default_answer.value + else: + timeout = None + timeout_answer = "" + message_prompt = f'\n{message} \nPress {allowed_answers}' + if default_answer and timeout: + message_prompt += ( + f". Auto-select {timeout_answer} in {timeout} seconds " + f"(add `--answer {default_answer.value}` to avoid delay next time)" + ) + message_prompt += ": " user_status = inputimeout( - prompt=f'\n{message} \nPress {allowed_answers}' - + (f" in {timeout} seconds: " if timeout is not None else ": "), + prompt=message_prompt, timeout=timeout, ) + if user_status == '': + if default_answer: + return default_answer + else: + continue if user_status.upper() in ['Y', 'YES']: return Answer.YES elif user_status.upper() in ['N', 'NO']: @@ -75,9 +96,10 @@ def user_confirm( else: print(f"Wrong answer given {user_status}. Should be one of {allowed_answers}. Try again.") except TimeoutOccurred: - if default_answer is not None: + if default_answer: return default_answer - print(f"Timeout after {timeout} seconds. Try again.") + # timeout should only occur when default_answer is set so this should never happened + continue except KeyboardInterrupt: if quit_allowed: return Answer.QUIT diff --git a/dev/breeze/src/airflow_breeze/utils/md5_build_check.py b/dev/breeze/src/airflow_breeze/utils/md5_build_check.py index 0071c0c8e3..eedec9296d 100644 --- a/dev/breeze/src/airflow_breeze/utils/md5_build_check.py +++ b/dev/breeze/src/airflow_breeze/utils/md5_build_check.py @@ -98,7 +98,8 @@ def md5sum_check_if_build_is_needed(md5sum_cache_dir: Path, verbose: bool) -> bo modified_files, not_modified_files = calculate_md5_checksum_for_files(md5sum_cache_dir, update=False) if len(modified_files) > 0: get_console().print( - '[warning]The following files are modified since last time image was built: [/]\n\n' + f'[warning]The following important files are modified in {AIRFLOW_SOURCES_ROOT} ' + f'since last time image was built: [/]\n\n' ) for file in modified_files: get_console().print(f" * [info]{file}[/]") diff --git a/dev/breeze/src/airflow_breeze/utils/path_utils.py b/dev/breeze/src/airflow_breeze/utils/path_utils.py index d3003305f8..74ae2831a8 100644 --- a/dev/breeze/src/airflow_breeze/utils/path_utils.py +++ b/dev/breeze/src/airflow_breeze/utils/path_utils.py @@ -26,6 +26,7 @@ from pathlib import Path from typing import Optional from airflow_breeze import NAME +from airflow_breeze.utils.confirm import set_forced_answer from airflow_breeze.utils.console import get_console from airflow_breeze.utils.reinstall import ( ask_to_reinstall_breeze, @@ -120,6 +121,16 @@ def get_used_sources_setup_metadata_hash() -> str: return get_sources_setup_metadata_hash(get_used_airflow_sources()) +def set_forced_answer_for_upgrade_check(): + """When we run upgrade check --answer is not parsed yet, so we need to guess it.""" + if "--answer n" in " ".join(sys.argv).lower() or os.environ.get('ANSWER', '').lower().startswith("n"): + set_forced_answer("no") + if "--answer y" in " ".join(sys.argv).lower() or os.environ.get('ANSWER', '').lower().startswith("y"): + set_forced_answer("yes") + if "--answer q" in " ".join(sys.argv).lower() or os.environ.get('ANSWER', '').lower().startswith("q"): + set_forced_answer("quit") + + def print_warning_if_setup_changed() -> bool: """ Prints warning if detected airflow sources are not the ones that Breeze was installed with. @@ -132,7 +143,9 @@ def print_warning_if_setup_changed() -> bool: if installation_sources is not None: breeze_sources = installation_sources / "dev" / "breeze" warn_dependencies_changed() + set_forced_answer_for_upgrade_check() ask_to_reinstall_breeze(breeze_sources) + set_forced_answer(None) return True return False diff --git a/dev/breeze/src/airflow_breeze/utils/reinstall.py b/dev/breeze/src/airflow_breeze/utils/reinstall.py index ca869fd643..0df3a6a0ea 100644 --- a/dev/breeze/src/airflow_breeze/utils/reinstall.py +++ b/dev/breeze/src/airflow_breeze/utils/reinstall.py @@ -18,9 +18,10 @@ import subprocess import sys from pathlib import Path +from typing import Optional from airflow_breeze import NAME -from airflow_breeze.utils.confirm import Answer, user_confirm +from airflow_breeze.utils.confirm import STANDARD_TIMEOUT, Answer, user_confirm from airflow_breeze.utils.console import get_console @@ -41,15 +42,17 @@ def reinstall_breeze(breeze_sources: Path): sys.exit(0) -def ask_to_reinstall_breeze(breeze_sources: Path): +def ask_to_reinstall_breeze(breeze_sources: Path, timeout: Optional[int] = STANDARD_TIMEOUT): """ Ask the user to reinstall Breeze (and do so if confirmed). :param breeze_sources: breeze sources to reinstall Breeze from. + :param timeout to answer - if set (when upgrade is auxiliary) then default answer is no, + otherwise it is yes """ answer = user_confirm( f"Do you want to reinstall Breeze from {breeze_sources.parent.parent}?", - timeout=3, - default_answer=Answer.NO, + default_answer=Answer.NO if timeout else Answer.YES, + timeout=timeout, ) if answer == Answer.YES: reinstall_breeze(breeze_sources) diff --git a/dev/breeze/src/airflow_breeze/utils/run_utils.py b/dev/breeze/src/airflow_breeze/utils/run_utils.py index b215cb523b..1412655f2b 100644 --- a/dev/breeze/src/airflow_breeze/utils/run_utils.py +++ b/dev/breeze/src/airflow_breeze/utils/run_utils.py @@ -186,7 +186,9 @@ def get_filesystem_type(filepath): def instruct_build_image(python: str): """Print instructions to the user that they should build the image""" get_console().print(f'[warning]\nThe CI image for ' f'python version {python} may be outdated[/]\n') - print(f"\n[info]Please run at the earliest convenience:[/]\n\nbreeze build-image --python {python}\n\n") + get_console().print( + f"\n[info]Please run at the earliest convenience:[/]\n\nbreeze build-image --python {python}\n\n" + ) @contextlib.contextmanager
