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 ebc1f14db3 Unify approach for user questions asked in Breeze (#23335)
ebc1f14db3 is described below
commit ebc1f14db3a1b14f2535462e97a6407f48b19f7c
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.
---
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
