potiuk commented on a change in pull request #18439:
URL: https://github.com/apache/airflow/pull/18439#discussion_r721334385
##########
File path: airflow/utils/cli.py
##########
@@ -89,6 +90,8 @@ def wrapper(*args, **kwargs):
metrics = _build_metrics(f.__name__, args[0])
cli_action_loggers.on_pre_execution(**metrics)
try:
+ # Check and run migrations if necessary
+ check_and_run_migrations()
Review comment:
I tihnk really the ones that for sure should NOT have the check is the
`db` commmand (and all subcommands of it). The `db` command is the one that is
used to managed the database and having de-synchronized migration there is
"acceptable" really. Then you can either reset/upgrade or shell to running db
without worrying about the state of synchronization (because you can use those
commands to fix or inspect a problem if for example your migration fails).
I would also skip it for the "installation" inspection/helper ones
(`version`, `providers`, `plugins`, `cheat-sheet`) - there is `slight` risk
here - some of those commands 'could` work before without the DB and for
example we are running them to check if the "image" works fine, without
actually having a DB initialized. Those never access/modify the DB.
For all the rest, I think makes perfect sense to perform the check, as they
simply need the DB in the **right** version to run.
On one hand we could introduce a new decorator (like `action_logging`) but
it is better to explicitly exclude the "non-DB" commands rather than add
decorators for everything else. So maybe renaming `action_logging` to more
generic (action_cli) decorator and adding `check_db=True` parameter and set
them to false for all `db, versions, providers, plugins, cheat-sheet` commands
makes more sense? I think we should rather explicitly exclude those commands
than do not need db migration check than include the check by adding new
decorator.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
