Skip to content

Commit

Permalink
Make passing build args explicit in ci/prod builds (apache#35768)
Browse files Browse the repository at this point in the history 
When building hte image, breeze converts some simple parameters
passed as breeze command (with autocompletion and explanation)
into much longer and more complex set of build args that are passed
to `docker build` command. The way how passing hte args worked so far
is that it was pretty implicit:

* **kwargs were used to ingest `click` flags
* parameters found as empty/None were filtered out from these
* Build*Params dataclass was created out of such kwargs dict
* argumenst from dataclass (with some customization) were
  converted to --build-arg (CAPITALIZED_PROPERTY_NAME)

This had a lot of implicitness and it was not easy to understand
whether the parameters passed were correct and how they passed
through this chain.

This change makes all the build arg much more explicit - without
kwargs and dictionary. Each CI/PROD build param has now a method
where it explicitly converts arguments into build-args - including
specifying which of those are optional (where you can actually
filter out Empty and None values) and which are required (where
an actual value is expected).

This PR also cleans up the click flags sequence and their
presence as well as the output of help command (they were grouped
with more related parameters)
  • Loading branch information
@potiuk
potiuk authored Nov 21, 2023
1 parent 7389782 commit 50e0b92
Show file tree
Hide file tree
Showing 15 changed files with 619 additions and 457 deletions.
119 changes: 87 additions & 32 deletions dev/breeze/src/airflow_breeze/commands/ci_image_commands.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import subprocess
import sys
import time
from copy import deepcopy
from functools import partial
from pathlib import Path
from typing import TYPE_CHECKING, Any, Callable
Expand All @@ -32,10 +33,10 @@
from airflow_breeze.utils.ci_group import ci_group
from airflow_breeze.utils.click_utils import BreezeGroup
from airflow_breeze.utils.common_options import (
option_additional_airflow_extras,
option_additional_dev_apt_command,
option_additional_dev_apt_deps,
option_additional_dev_apt_env,
option_additional_extras,
option_additional_pip_install_flags,
option_additional_python_deps,
option_airflow_constraints_location,
Expand All @@ -53,7 +54,6 @@
option_docker_cache,
option_dry_run,
option_eager_upgrade_additional_requirements,
option_force_build,
option_github_repository,
option_github_token,
option_image_name,
Expand All @@ -77,7 +77,7 @@
option_upgrade_to_newer_dependencies,
option_verbose,
option_verify,
option_version_suffix_for_pypi,
option_version_suffix_for_pypi_ci,
option_wait_for_image,
)
from airflow_breeze.utils.confirm import STANDARD_TIMEOUT, Answer, user_confirm
Expand All @@ -99,7 +99,6 @@
from airflow_breeze.utils.registry import login_to_github_docker_registry
from airflow_breeze.utils.run_tests import verify_an_image
from airflow_breeze.utils.run_utils import (
filter_out_none,
fix_group_permissions,
instruct_build_image,
is_repo_rebased,
Expand Down Expand Up @@ -224,12 +223,6 @@ def get_exitcode(status: int) -> int:
@ci_image.command(name="build")
@option_python
@option_debian_version
@option_run_in_parallel
@option_parallelism
@option_skip_cleanup
@option_debug_resources
@option_include_success_outputs
@option_python_versions
@option_upgrade_to_newer_dependencies
@option_upgrade_on_failure
@option_platform_multiple
Expand All @@ -239,18 +232,16 @@ def get_exitcode(status: int) -> int:
@option_prepare_buildx_cache
@option_push
@option_install_providers_from_sources
@option_additional_extras
@option_additional_airflow_extras
@option_additional_dev_apt_deps
@option_additional_python_deps
@option_additional_dev_apt_command
@option_additional_dev_apt_env
@option_builder
@option_build_progress
@option_build_timeout_minutes
@option_commit_sha
@option_dev_apt_command
@option_dev_apt_deps
@option_force_build
@option_python_image
@option_eager_upgrade_additional_requirements
@option_airflow_constraints_location
Expand All @@ -259,19 +250,58 @@ def get_exitcode(status: int) -> int:
@option_tag_as_latest
@option_additional_pip_install_flags
@option_github_repository
@option_version_suffix_for_pypi
@option_version_suffix_for_pypi_ci
@option_build_timeout_minutes
@option_run_in_parallel
@option_parallelism
@option_skip_cleanup
@option_debug_resources
@option_include_success_outputs
@option_python_versions
@option_verbose
@option_dry_run
@option_answer
def build(
# Build options
python: str,
debian_version: str,
upgrade_to_newer_dependencies: bool,
upgrade_on_failure: bool,
platform: str | None,
github_token: str | None,
docker_cache: str,
image_tag: str,
prepare_buildx_cache: bool,
push: bool,
install_providers_from_sources: bool,
additional_airflow_extras: str | None,
additional_dev_apt_deps: str | None,
additional_python_deps: str | None,
additional_dev_apt_command: str | None,
additional_dev_apt_env: str | None,
builder: str,
build_progress: str,
commit_sha: str | None,
dev_apt_command: str | None,
dev_apt_deps: str | None,
eager_upgrade_additional_requirements: str | None,
airflow_constraints_location: str | None,
airflow_constraints_mode: str,
airflow_constraints_reference: str,
tag_as_latest: bool,
additional_pip_install_flags: str | None,
github_repository: str,
python_image: str | None,
version_suffix_for_pypi: str,
# Parallel building
run_in_parallel: bool,
parallelism: int,
skip_cleanup: bool,
debug_resources: bool,
include_success_outputs,
python_versions: str,
# Other options
build_timeout_minutes: int | None,
**kwargs: dict[str, Any],
):
"""Build CI image. Include building multiple images for all python versions."""

Expand Down Expand Up @@ -306,16 +336,51 @@ def run_build(ci_image_params: BuildCiParams) -> None:

perform_environment_checks()
check_remote_ghcr_io_commands()
parameters_passed = filter_out_none(**kwargs)
parameters_passed["force_build"] = True
fix_group_permissions()
base_build_params = BuildCiParams(
force_build=True,
python=python,
debian_version=debian_version,
upgrade_to_newer_dependencies=upgrade_to_newer_dependencies,
upgrade_on_failure=upgrade_on_failure,
github_token=github_token,
docker_cache=docker_cache,
image_tag=image_tag,
prepare_buildx_cache=prepare_buildx_cache,
push=push,
install_providers_from_sources=install_providers_from_sources,
additional_airflow_extras=additional_airflow_extras,
additional_python_deps=additional_python_deps,
additional_dev_apt_command=additional_dev_apt_command,
additional_dev_apt_env=additional_dev_apt_env,
builder=builder,
build_progress=build_progress,
commit_sha=commit_sha,
dev_apt_command=dev_apt_command,
dev_apt_deps=dev_apt_deps,
eager_upgrade_additional_requirements=eager_upgrade_additional_requirements,
airflow_constraints_location=airflow_constraints_location,
airflow_constraints_mode=airflow_constraints_mode,
airflow_constraints_reference=airflow_constraints_reference,
tag_as_latest=tag_as_latest,
additional_pip_install_flags=additional_pip_install_flags,
github_repository=github_repository,
python_image=python_image,
version_suffix_for_pypi=version_suffix_for_pypi,
)
if platform:
base_build_params.platform = platform
if additional_dev_apt_deps:
# For CI image we only set additional_dev_apt_deps when we explicitly pass it
base_build_params.additional_dev_apt_deps = additional_dev_apt_deps

if run_in_parallel:
python_version_list = get_python_version_list(python_versions)
params_list: list[BuildCiParams] = []
for python in python_version_list:
params = BuildCiParams(**parameters_passed)
params.python = python
params_list.append(params)
build_params = deepcopy(base_build_params)
build_params.python = python
params_list.append(build_params)
prepare_for_building_ci_image(params=params_list[0])
run_build_in_parallel(
image_params_list=params_list,
Expand All @@ -326,9 +391,8 @@ def run_build(ci_image_params: BuildCiParams) -> None:
debug_resources=debug_resources,
)
else:
params = BuildCiParams(**parameters_passed)
prepare_for_building_ci_image(params=params)
run_build(ci_image_params=params)
prepare_for_building_ci_image(params=base_build_params)
run_build(ci_image_params=base_build_params)


@ci_image.command(name="pull")
Expand Down Expand Up @@ -550,13 +614,6 @@ def run_build_ci_image(
:param ci_image_params: CI image parameters
:param output: output redirection
"""
if not ci_image_params.version_suffix_for_pypi:
# We need that to handle the >= 2.7.0 limit we have for openlineage provider at least until
# Airflow 2.7.0 release is out, in order to avoid conflicting dependencies while building the image
# We are setting version_suffix_for_pypi to dev0 for CI builds where cache is prepared, so in
# order to have the cache used effectively, we should also locally force the version_suffix_for_pypi
# to dev0. We might evan leave it as default value in the future (to be decided after 2.7.0 release)
ci_image_params.version_suffix_for_pypi = "dev0"
if (
ci_image_params.is_multi_platform()
and not ci_image_params.push
Expand Down Expand Up @@ -645,8 +702,6 @@ def rebuild_or_pull_ci_image_if_needed(command_params: ShellParams | BuildCiPara
Rebuilds CI image if needed and user confirms it.
:param command_params: parameters of the command to execute
"""
build_ci_image_check_cache = Path(
BUILD_CACHE_DIR, command_params.airflow_branch, f".built_{command_params.python}"
Expand Down
29 changes: 19 additions & 10 deletions dev/breeze/src/airflow_breeze/commands/ci_image_commands_config.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"--image-tag",
"--tag-as-latest",
"--docker-cache",
"--force-build",
"--version-suffix-for-pypi",
"--build-progress",
],
},
Expand All @@ -51,24 +51,33 @@
],
},
{
"name": "Advanced options (for power users)",
"name": "Advanced build options (for power users)",
"options": [
"--debian-version",
"--python-image",
"--commit-sha",
"--additional-pip-install-flags",
"--install-providers-from-sources",
],
},
{
"name": "Selecting constraint location (for power users)",
"options": [
"--airflow-constraints-location",
"--airflow-constraints-mode",
"--airflow-constraints-reference",
"--python-image",
"--additional-python-deps",
],
},
{
"name": "Choosing dependencies and extras (for power users)",
"options": [
"--additional-airflow-extras",
"--additional-pip-install-flags",
"--additional-dev-apt-deps",
"--additional-dev-apt-env",
"--additional-dev-apt-command",
"--additional-python-deps",
"--dev-apt-deps",
"--additional-dev-apt-deps",
"--dev-apt-command",
"--version-suffix-for-pypi",
"--commit-sha",
"--additional-dev-apt-command",
"--additional-dev-apt-env",
],
},
{
Expand Down
Loading

0 comments on commit 50e0b92

Please sign in to comment.