Skip to content

Commit

Permalink
Cleaner default output when breeze starts (apache#23341)
Browse files Browse the repository at this point in the history
There was a bit of noise printed when Breeze started:

* information about branch/python/image/backend used
* information about actions performed (like fixing permissions)
* information that docke image build is not needed
* warnings about missing variables

This PR marks all the messages as "info" and only prints them
when --verbose flag is used and it adds default values for the
variables that generated warnings.
potiuk authored Apr 28, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
1 parent f5f9c58 commit dd7002d
Showing 9 changed files with 29 additions and 29 deletions.
12 changes: 7 additions & 5 deletions dev/breeze/src/airflow_breeze/build_image/ci/build_ci_image.py
Original file line number Diff line number Diff line change
@@ -49,7 +49,7 @@
)


def should_we_run_the_build(build_ci_params: BuildCiParams) -> bool:
def should_we_run_the_build(build_ci_params: BuildCiParams, verbose: bool) -> bool:
"""
Check if we should run the build based on what files have been modified since last build and answer from
the user.
@@ -59,11 +59,14 @@ def should_we_run_the_build(build_ci_params: BuildCiParams) -> bool:
* Builds Image/Skips/Quits depending on the answer
:param build_ci_params: parameters for the build
:param verbose: should we get verbose information
"""
# We import those locally so that click autocomplete works
from inputimeout import TimeoutOccurred

if not md5sum_check_if_build_is_needed(md5sum_cache_dir=build_ci_params.md5sum_cache_dir):
if not md5sum_check_if_build_is_needed(
md5sum_cache_dir=build_ci_params.md5sum_cache_dir, verbose=verbose
):
return False
try:
answer = user_confirm(message="Do you want to build image?", timeout=5, default_answer=Answer.NO)
@@ -123,7 +126,7 @@ def build_ci_image(
:param with_ci_group: whether to wrap the build in CI logging group
:param ci_image_params: CI image parameters
"""
fix_group_permissions()
fix_group_permissions(verbose=verbose)
if verbose or dry_run:
get_console().print(
f"\n[info]Building CI image of airflow from {AIRFLOW_SOURCES_ROOT} "
@@ -133,9 +136,8 @@ def build_ci_image(
f"Build CI image for Python {ci_image_params.python} " f"with tag: {ci_image_params.image_tag}",
enabled=with_ci_group,
):
ci_image_params.print_info()
if not ci_image_params.force_build and not ci_image_params.upgrade_to_newer_dependencies:
if not should_we_run_the_build(build_ci_params=ci_image_params):
if not should_we_run_the_build(build_ci_params=ci_image_params, verbose=verbose):
return 0, f"Image build: {ci_image_params.python}"
run_command(
["docker", "rmi", "--no-prune", "--force", ci_image_params.airflow_image_name],
Original file line number Diff line number Diff line change
@@ -23,7 +23,6 @@

from airflow_breeze.branch_defaults import AIRFLOW_BRANCH, DEFAULT_AIRFLOW_CONSTRAINTS_BRANCH
from airflow_breeze.global_constants import get_airflow_version
from airflow_breeze.utils.console import get_console
from airflow_breeze.utils.path_utils import BUILD_CACHE_DIR


@@ -148,9 +147,6 @@ def extra_docker_build_flags(self) -> List[str]:
def md5sum_cache_dir(self) -> Path:
return Path(BUILD_CACHE_DIR, self.airflow_branch, self.python, "CI")

def print_info(self):
get_console().print(f"CI Image: {self.airflow_version} Python: {self.python}.")


REQUIRED_CI_IMAGE_ARGS = [
"python_base_image",
Original file line number Diff line number Diff line change
@@ -143,7 +143,7 @@ def build_production_image(
:param with_ci_group: whether to wrap the build in CI logging group
:param prod_image_params: PROD image parameters
"""
fix_group_permissions()
fix_group_permissions(verbose=verbose)
if verbose or dry_run:
get_console().print(
f"\n[info]Building PROD image of airflow from {AIRFLOW_SOURCES_ROOT} "
@@ -154,7 +154,6 @@ def build_production_image(
f"with tag: {prod_image_params.image_tag}",
enabled=with_ci_group,
):
prod_image_params.print_info()
if prod_image_params.cleanup_context:
clean_docker_context_files(verbose=verbose, dry_run=dry_run)
check_docker_context_files(prod_image_params.install_packages_from_context)
Original file line number Diff line number Diff line change
@@ -238,9 +238,6 @@ def airflow_image_date_created(self):
def airflow_image_readme_url(self):
return "https://raw.githubusercontent.com/apache/airflow/main/docs/docker-stack/README.md"

def print_info(self):
get_console().print(f"CI Image: {self.airflow_version} Python: {self.python}.")

@property
def airflow_pre_cached_pip_packages(self) -> str:
airflow_pre_cached_pip = 'true'
3 changes: 2 additions & 1 deletion dev/breeze/src/airflow_breeze/shell/enter_shell.py
Original file line number Diff line number Diff line change
@@ -122,7 +122,8 @@ def run_shell_with_build_image_checks(
)
ci_image_params = BuildCiParams(python=shell_params.python, upgrade_to_newer_dependencies=False)
if build_ci_image_check_cache.exists():
get_console().print(f'[info]{shell_params.the_image_type} image already built locally.[/]')
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.[/]'
15 changes: 8 additions & 7 deletions dev/breeze/src/airflow_breeze/shell/shell_params.py
Original file line number Diff line number Diff line change
@@ -143,13 +143,14 @@ def sqlite_url(self) -> str:
return sqlite_url

def print_badge_info(self):
get_console().print(f'Use {self.the_image_type} image')
get_console().print(f'Branch Name: {self.airflow_branch}')
get_console().print(f'Docker Image: {self.airflow_image_name_with_tag}')
get_console().print(f'Airflow source version:{self.airflow_version}')
get_console().print(f'Python Version: {self.python}')
get_console().print(f'Backend: {self.backend} {self.backend_version}')
get_console().print(f'Airflow used at runtime: {self.use_airflow_version}')
if self.verbose:
get_console().print(f'[info]Use {self.the_image_type} image[/]')
get_console().print(f'[info]Branch Name: {self.airflow_branch}[/]')
get_console().print(f'[info]Docker Image: {self.airflow_image_name_with_tag}[/]')
get_console().print(f'[info]Airflow source version:{self.airflow_version}[/]')
get_console().print(f'[info]Python Version: {self.python}[/]')
get_console().print(f'[info]Backend: {self.backend} {self.backend_version}[/]')
get_console().print(f'[info]Airflow used at runtime: {self.use_airflow_version}[/]')

@property
def compose_files(self):
3 changes: 2 additions & 1 deletion dev/breeze/src/airflow_breeze/utils/docker_command_utils.py
Original file line number Diff line number Diff line change
@@ -440,6 +440,7 @@ def update_expected_environment_variables(env: Dict[str, str]) -> None:
:param env: environment variables to update with missing values if not set.
"""
set_value_to_default_if_not_set(env, 'ANSWER', "")
set_value_to_default_if_not_set(env, 'AIRFLOW_EXTRAS', "")
set_value_to_default_if_not_set(env, 'BREEZE', "true")
set_value_to_default_if_not_set(env, 'CI', "false")
set_value_to_default_if_not_set(env, 'CI_BUILD_ID', "0")
@@ -475,9 +476,9 @@ def update_expected_environment_variables(env: Dict[str, str]) -> None:
set_value_to_default_if_not_set(env, 'TEST_TYPE', "")
set_value_to_default_if_not_set(env, 'UPGRADE_TO_NEWER_DEPENDENCIES', "false")
set_value_to_default_if_not_set(env, 'USE_PACKAGES_FROM_DIST', "false")
set_value_to_default_if_not_set(env, 'USE_PACKAGES_FROM_DIST', "false")
set_value_to_default_if_not_set(env, 'VERBOSE', "false")
set_value_to_default_if_not_set(env, 'VERBOSE_COMMANDS', "false")
set_value_to_default_if_not_set(env, 'VERSION_SUFFIX_FOR_PYPI', "")
set_value_to_default_if_not_set(env, 'WHEEL_VERSION', "0.36.2")


10 changes: 6 additions & 4 deletions dev/breeze/src/airflow_breeze/utils/md5_build_check.py
Original file line number Diff line number Diff line change
@@ -86,11 +86,12 @@ def calculate_md5_checksum_for_files(
return modified_files, not_modified_files


def md5sum_check_if_build_is_needed(md5sum_cache_dir: Path) -> bool:
def md5sum_check_if_build_is_needed(md5sum_cache_dir: Path, verbose: bool) -> bool:
"""
Checks if build is needed based on whether important files were modified.
:param md5sum_cache_dir: directory where cached md5 sums are stored
:param verbose: should we print verbose information
:return: True if build is needed.
"""
build_needed = False
@@ -104,9 +105,10 @@ def md5sum_check_if_build_is_needed(md5sum_cache_dir: Path) -> bool:
get_console().print('\n[warning]Likely CI image needs rebuild[/]\n')
build_needed = True
else:
get_console().print(
'Docker image build is not needed for CI build as no important files are changed!'
)
if verbose:
get_console().print(
'[info]Docker image build is not needed for CI build as no important files are changed![/]'
)
return build_needed


5 changes: 3 additions & 2 deletions dev/breeze/src/airflow_breeze/utils/run_utils.py
Original file line number Diff line number Diff line change
@@ -211,9 +211,10 @@ def change_directory_permission(directory_to_fix: Path):


@working_directory(AIRFLOW_SOURCES_ROOT)
def fix_group_permissions():
def fix_group_permissions(verbose: bool):
"""Fixes permissions of all the files and directories that have group-write access."""
get_console().print("[info]Fixing group permissions[/]")
if verbose:
get_console().print("[info]Fixing group permissions[/]")
files_to_fix_result = run_command(['git', 'ls-files', './'], capture_output=True, text=True)
if files_to_fix_result.returncode == 0:
files_to_fix = files_to_fix_result.stdout.strip().split('\n')

0 comments on commit dd7002d

Please sign in to comment.