Skip to content

Commit

Permalink
Modify breeze commands and flags for standalone shell commands (#35862)
Browse files Browse the repository at this point in the history
Breeze `shell` command has been foreseen eventually to also allow
single commands in CI and pre-commit, and we are preparing it to
be able to do so with all the bells and whistles. There are a
few improvements needed to be able to do it smoothly.

There are a few things that were preventing using `breeze shell` as
general-purpose commmand executor:

* we used defailt `docker-compose` project for running compose runs
  (derived from `docker-compose` parent folder of the `basy.yml` compose
  file. If we want to run standalone commands, we need to run them in
  separate projects in order to allow to run them while `breeze` shell
  is already running

* when runing command in pre-commit hook, there is no terminal available
  because this command is run deep under-the-hood in git internals and
  it is run as command detached from the terminal it runs in. For that
  automated terminal allocation strategy used by docker compose was
  not working

* breeze prints a lot of diagnostics of what's going on when you run
  the entrypoint by default - including diagnostics information
  printed by docker compose and automated docker builds. This is
  polluting the output of automated scripts (especially in pre-commits)

* breeze command checks by defalt if image needs to be upgraded and
  prints information about it and allows the user to trigger the
  upgrade by answering "yes" for 10 seconds. This add extra delay
  and unnecessary information when running the scripts in pre-commits
  and generally in unattended mode.

* Breeze CI entrypoint uses additional environment initialization
  when entering the container - it waits until DBs are available
  and performs few other initializaiton checks (for example
  loading scripts from `init.sh`. All that is not needed and should
  be skipped when running standalone scripts.

* Sometimes it's not obious what internal commands are run by breeze
  shell - and it's difficult to debug/diagnose it. We run CI commands
  with `--verbose` flags so those command should not be printed even
  if `--verbose` flag is used though as they are polluting the output.

* By default some flags are persistently stored after used (python
  version, backend, backend version) and they are "cached" - they
  are automatically used next time you start breeze - to keep the
  environment you use for the last time. Those shoudl not be
  persisted when running standalone command and want to override Python
  version or backend only in the command you want to execute.

The PR implements the following changes to improve those aspects:

* Allow specifying `--project-name` when running `breeze`, `breze shell`
  `breeze start-airflow` and `breeze down` commands. Default project is
  `breeze` now and user can use any project but predefined ones are
  also `pre-commit` (to be used in pre-commits) and legacy
  `docker-compose` if one would like to use `breeze down` to shut down
  some remaining containers from the legacy project.

* Add `--restart` (aliased by `--remove-orphans`) flag that might be
  used when starting the command, which will also remove orphan
  containers (for example postgres container running while we are
  switching to sqlite).

* Add `--tty` option ("auto", "disabled", "enabled") in `breeze shell`
  command. This flag allows to determine strategy of terminal allocation
  by the underlying docker-compose flags.

* Add `--quiet` flag that silences the output of entrypoint diagnostics
  including the output of docker-compose and docker build when they
  are running as part of the command.

NOTE: We also need to upgrade min docker-compose and docker version
as we are using features available only in recent versions of these
that allow to suppress some output.

* Add `--skip-image-upgrade-check` in `breeze shell` to avoid checking
  if upgrade is needed. However if the image is not present, the latest
  version will be upgraded anyway - this allows to run pre-commits even
  if breeze has not been used before or `docker system prune -a`
  was used. We had `--skip-image-check` before, but we rename it to
  explain what check we are talking about.

* Add `--skip-environment-initialization` in `breeze shell` to skip
  all initialization happening for interactive breeze entering.

* Add `--verbose-commands` flag in `breeze shell`. For a long time
  we had `VERBOSE_COMMANDS` env variable support doing it (and printing
  commands as they are executed) but adding it explicitly as flag makes
  it easier discoverable and usable.

* Add `SKIP_SAVING_CHOICES` environment variable. When this variable
  is set, the choices you specify when runing breeze command are
  not stored in cache and only used for the particular command you
  execute.

Few other cleanups and minor fixes  were also implemented as part of this
change:

* the click decorators and corresponding parameters in
  breeze/shell/start-airflow commands have been sorted to easier manage
  them (adding/updating the option became a real pain for those as they
  have a lot of them)

* version_suffix_for_pypi default for CI image has been set up to dev0
  to make automated upgrade after pressing `Y` use the same cache
  as `docker ci-image build`

* added PYTHONWARNINGS to be forwardeable from host environment to
  container - which allows to control which warnings to display.

* only print "Github Actions" group end when we are running inside
  GitHub Actions.

* switch from distutls Versions to packaging Version - used
  to check pre-commit version (better Python 3.12 compatibility)

* clarified that executor flag for "shell" command can accept more
  values than the one in "start-airflow"

* when config options are not synchronized with "rich-click" help
  groups, proper location is displayed in error messsages (it asked
  to fix the issue in non-existing files).
  • Loading branch information
potiuk authored Nov 26, 2023
1 parent fa94ee9 commit 4495de7
Show file tree
Hide file tree
Showing 36 changed files with 930 additions and 528 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -608,7 +608,7 @@ jobs:
- name: "Static checks: basic checks only"
run: >
breeze static-checks --show-diff-on-failure --color always --initialize-environment
--skip-image-check --commit-ref "${{ github.sha }}"
--skip-image-upgrade-check --commit-ref "${{ github.sha }}"
env:
VERBOSE: "false"
SKIP_IMAGE_PRE_COMMITS: "true"
Expand Down
7 changes: 6 additions & 1 deletion BREEZE.rst
Original file line number Diff line number Diff line change
Expand Up @@ -458,8 +458,13 @@ The choices you make are persisted in the ``./.build/`` cache directory so that
them when you run the script. You can delete the ``.build/`` directory in case you want to restore the
default settings.

You can also run breeze with ``SKIP_SAVING_CHOICES`` to non-empty value and breeze invocation will not save
used cache value to cache - this is useful when you run non-interactive scripts with ``breeze shell`` and
want to - for example - force Python version used only for that execution without changing the Python version
that user used last time.

You can see which value of the parameters that can be stored persistently in cache marked with >VALUE<
in the help of the commands.
in the help of the commands (for example in output of ``breeze config --help``).

Building the documentation
--------------------------
Expand Down
4 changes: 4 additions & 0 deletions CI.rst
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,10 @@ those via command line flags passed to ``breeze`` command.
| | | | | |
| | | | | \* set to true in pre-commits |
+-----------------------------------------+-------------+--------------+------------+-------------------------------------------------+
| ``SKIP_IMAGE_UPGRADE_CHECK`` | false\* | false\* | false\* | Skip checking if image should be upgraded |
| | | | | |
| | | | | \* set to true in pre-commits |
+-----------------------------------------+-------------+--------------+------------+-------------------------------------------------+
| ``SKIP_PROVIDER_TESTS`` | false\* | false\* | false\* | Skip running provider integration tests |
+-----------------------------------------+-------------+--------------+------------+-------------------------------------------------+
| ``SKIP_SSH_SETUP`` | false\* | false\* | false\* | Skip setting up SSH server for tests. |
Expand Down
10 changes: 10 additions & 0 deletions dev/breeze/src/airflow_breeze/commands/ci_image_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,16 @@ def ci_image():

def check_if_image_building_is_needed(ci_image_params: BuildCiParams, output: Output | None) -> bool:
"""Starts building attempt. Returns false if we should not continue"""
result = run_command(
["docker", "inspect", ci_image_params.airflow_image_name_with_tag],
capture_output=True,
text=True,
check=False,
)
if result.returncode != 0:
return True
if ci_image_params.skip_image_upgrade_check:
return False
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):
return False
Expand Down
Loading

0 comments on commit 4495de7

Please sign in to comment.