You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2023/11/26 13:45:22 UTC
[PR] Modify breeze commands and flags for standalone shell commands [airflow]
potiuk opened a new pull request, #35862:
URL: https://github.com/apache/airflow/pull/35862
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.
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.
* 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.
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).
<!--
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.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify breeze commands and flags for standalone shell commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #35862:
URL: https://github.com/apache/airflow/pull/35862#issuecomment-1826877771
> First I was fearing the size to review - until I realized that 50% was boilerplate code changes and 50% SVG image generated :-D LGTM!
Precisely :).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify breeze commands and flags for standalone shell commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #35862:
URL: https://github.com/apache/airflow/pull/35862
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify breeze commands and flags for standalone shell commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #35862:
URL: https://github.com/apache/airflow/pull/35862#issuecomment-1826790086
This is a logically-separated PR from #35830 as discussed in https://github.com/apache/airflow/pull/35830#pullrequestreview-1748817163
It's mostly revolving about making `breeze shell` command more usable as standalone "driver" to execute arbitrary commands. It's quite a bit smaller than #35830 and focuses only on making such execution possible - once this is complete, I think I will just rebase the #35830 and what will be left is actually switching to the new `breeze shell` command as execution driver by the few pre-commits we have already using breeze code in a `twisted` way.
I still left a few minor "loosely-related" fixes. I could probably extract out a few of those in separate PRs if you think it makes sense ("Few other cleanups and minor fixes were also implemented as part of this change" above). But it would inflate number of PRs and since it is just `dev env`, It's not going to be cherry-picked individually, so I'd rather lave them in - there are one-liner fixes that are anyhow done close to change already being implemented.
But if needed I can separate them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Modify breeze commands and flags for standalone shell commands [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #35862:
URL: https://github.com/apache/airflow/pull/35862#issuecomment-1826794060
BTW. Even with those small fixes adde - it should be FAR easier to review than https://github.com/apache/airflow/pull/35830
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org