Consistent DB upgrade/downgrade arguments (#22537)

This is a follow up to #22102, and be forewarned, this might be a bikeshed. If this gets contentious at all, I'll just close it and move on.

I think it's a little bit easier for users to have consistent flags/arguments for the `airflow db upgrade` and `airflow db downgrade` commands. This PR just tweaks the argument processing to expect `--to-revision` and `--to-version` instead of `--revision` and `--version`, respectively.

That change makes the arguments to those commands more consistent with the `--from-revision` and `--from-version` arguments. Doing so also avoids overloading the `--version` flag, which is usually a flag that prints out the version information of the command itself (eg: Airflow's version, which is available via `airflow version`).

An argument against this change is that the `--to-...` arguments can be understood to be implied, like this:

```bash
airflow db upgrade --from-version 10.15.8  # Upgrade from 10.15.8 to the current Airflow version
```

and this means that you do not necessarily need to always specify the `--to-...` arguments. By having both `--to-` and `--from-` arguments, users might think that they always need to specify both a `--to-` and `--from-` argument.

I also fixed an unrelated grammar typo, which corrects the grammar used to log the operation.

GitOrigin-RevId: 60d90896486cc3d9f1fc0029ca9833c7d561caa4

airflow/cli/cli_parser.py

@@ -524,27 +524,27 @@ def string_lower_type(val):
     default=60,
 )
 ARG_DB_VERSION__UPGRADE = Arg(
-    ("-n", "--version"),
+    ("-n", "--to-version"),
     help=(
         "(Optional) The airflow version to upgrade to. Note: must provide either "
-        "`--revision` or `--version`."
+        "`--to-revision` or `--to-version`."
     ),
 )
 ARG_DB_REVISION__UPGRADE = Arg(
-    ("-r", "--revision"),
+    ("-r", "--to-revision"),
     help="(Optional) If provided, only run migrations up to and including this Alembic revision.",
 )
 ARG_DB_VERSION__DOWNGRADE = Arg(
-    ("-n", "--version"),
+    ("-n", "--to-version"),
     help="(Optional) If provided, only run migrations up to this version.",
 )
 ARG_DB_FROM_VERSION = Arg(
     ("--from-version",),
     help="(Optional) If generating sql, may supply a *from* version",
 )
 ARG_DB_REVISION__DOWNGRADE = Arg(
-    ("-r", "--revision"),
-    help="The Alembic revision to downgrade to. Note: must provide either `--revision` or `--version`.",
+    ("-r", "--to-revision"),
+    help="The Alembic revision to downgrade to. Note: must provide either `--to-revision` or `--to-version`.",
 )
 ARG_DB_FROM_REVISION = Arg(
     ("--from-revision",),
@@ -1407,7 +1407,7 @@ class GroupCommand(NamedTuple):
         help="Downgrade the schema of the metadata database.",
         description=(
             "Downgrade the schema of the metadata database. "
-            "You must provide either `--revision` or `--version`. "
+            "You must provide either `--to-revision` or `--to-version`. "
             "To print but not execute commands, use option `--show-sql-only`. "
             "If using options `--from-revision` or `--from-version`, you must also use `--show-sql-only`, "
             "because if actually *running* migrations, we should only migrate from the *current* Alembic "

airflow/cli/commands/db_command.py

@@ -49,15 +49,15 @@ def resetdb(args):
 def upgradedb(args):
     """Upgrades the metadata database"""
     print("DB: " + repr(settings.engine.url))
-    if args.revision and args.version:
-        raise SystemExit("Cannot supply both `--revision` and `--version`.")
+    if args.to_revision and args.to_version:
+        raise SystemExit("Cannot supply both `--to-revision` and `--to-version`.")
     if args.from_version and args.from_revision:
         raise SystemExit("Cannot supply both `--from-revision` and `--from-version`")
     if (args.from_revision or args.from_version) and not args.show_sql_only:
         raise SystemExit(
             "Args `--from-revision` and `--from-version` may only be used with `--show-sql-only`"
         )
-    revision = None
+    to_revision = None
     from_revision = None
     if args.from_revision:
         from_revision = args.from_revision
@@ -68,49 +68,49 @@ def upgradedb(args):
         if not from_revision:
             raise SystemExit(f"Unknown version {args.from_version!r} supplied as `--from-version`.")
 
-    if args.version:
-        revision = REVISION_HEADS_MAP.get(args.version)
-        if not revision:
-            raise SystemExit(f"Upgrading to version {args.version} is not supported.")
-    elif args.revision:
-        revision = args.revision
+    if args.to_version:
+        to_revision = REVISION_HEADS_MAP.get(args.to_version)
+        if not to_revision:
+            raise SystemExit(f"Upgrading to version {args.to_version} is not supported.")
+    elif args.to_revision:
+        to_revision = args.to_revision
 
     if not args.show_sql_only:
         print("Performing upgrade with database " + repr(settings.engine.url))
     else:
         print("Generating sql for upgrade -- upgrade commands will *not* be submitted.")
 
-    db.upgradedb(to_revision=revision, from_revision=from_revision, show_sql_only=args.show_sql_only)
+    db.upgradedb(to_revision=to_revision, from_revision=from_revision, show_sql_only=args.show_sql_only)
     if not args.show_sql_only:
         print("Upgrades done")
 
 
 @cli_utils.action_cli(check_db=False)
 def downgrade(args):
     """Downgrades the metadata database"""
-    if args.revision and args.version:
-        raise SystemExit("Cannot supply both `revision` and `version`.")
+    if args.to_revision and args.to_version:
+        raise SystemExit("Cannot supply both `--to-revision` and `--to-version`.")
     if args.from_version and args.from_revision:
         raise SystemExit("`--from-revision` may not be combined with `--from-version`")
     if (args.from_revision or args.from_version) and not args.show_sql_only:
         raise SystemExit(
             "Args `--from-revision` and `--from-version` may only be used with `--show-sql-only`"
         )
-    if not (args.version or args.revision):
-        raise SystemExit("Must provide either revision or version.")
+    if not (args.to_version or args.to_revision):
+        raise SystemExit("Must provide either --to-revision or --to-version.")
     from_revision = None
     if args.from_revision:
         from_revision = args.from_revision
     elif args.from_version:
         from_revision = REVISION_HEADS_MAP.get(args.from_version)
         if not from_revision:
-            raise SystemExit(f"Unknown version {args.version!r} supplied as `--from-version`.")
-    if args.version:
-        revision = REVISION_HEADS_MAP.get(args.version)
-        if not revision:
-            raise SystemExit(f"Downgrading to version {args.version} is not supported.")
-    elif args.revision:
-        revision = args.revision
+            raise SystemExit(f"Unknown version {args.from_version!r} supplied as `--from-version`.")
+    if args.to_version:
+        to_revision = REVISION_HEADS_MAP.get(args.to_version)
+        if not to_revision:
+            raise SystemExit(f"Downgrading to version {args.to_version} is not supported.")
+    elif args.to_revision:
+        to_revision = args.to_revision
     if not args.show_sql_only:
         print("Performing downgrade with database " + repr(settings.engine.url))
     else:
@@ -125,7 +125,7 @@ def downgrade(args):
         ).upper()
         == "Y"
     ):
-        db.downgrade(to_revision=revision, from_revision=from_revision, show_sql_only=args.show_sql_only)
+        db.downgrade(to_revision=to_revision, from_revision=from_revision, show_sql_only=args.show_sql_only)
         if not args.show_sql_only:
             print("Downgrade complete")
     else:

airflow/utils/db.py

@@ -742,7 +742,7 @@ def check_and_run_migrations():
     if len(db_heads) < 1:
         db_command = initdb
         command_name = "init"
-        verb = "initialization"
+        verb = "initialize"
     elif source_heads != db_heads:
         db_command = upgradedb
         command_name = "upgrade"

scripts/ci/testing/run_offline_sql_test.sh

@@ -26,4 +26,4 @@ testing::get_docker_compose_local
 testing::setup_docker_compose_backend "offline-sql-test"
 # We test from 2.0.0 upwards
 testing::run_command_in_docker "offline-sql-test" "airflow db upgrade --from-version 2.0.0 -r heads --show-sql-only \
-    && airflow db downgrade --version 2.0.0 --show-sql-only -y"
+    && airflow db downgrade --to-version 2.0.0 --show-sql-only -y"

tests/cli/commands/test_db_command.py

@@ -56,33 +56,33 @@ def test_cli_check_migrations(self, mock_wait_for_migrations):
         [
             ([], dict(to_revision=None, from_revision=None, show_sql_only=False)),
             (['--show-sql-only'], dict(to_revision=None, from_revision=None, show_sql_only=True)),
-            (['--revision', 'abc'], dict(to_revision='abc', from_revision=None, show_sql_only=False)),
+            (['--to-revision', 'abc'], dict(to_revision='abc', from_revision=None, show_sql_only=False)),
             (
-                ['--revision', 'abc', '--show-sql-only'],
+                ['--to-revision', 'abc', '--show-sql-only'],
                 dict(to_revision='abc', from_revision=None, show_sql_only=True),
             ),
             (
-                ['--version', '2.2.2'],
+                ['--to-version', '2.2.2'],
                 dict(to_revision='7b2661a43ba3', from_revision=None, show_sql_only=False),
             ),
             (
-                ['--version', '2.2.2', '--show-sql-only'],
+                ['--to-version', '2.2.2', '--show-sql-only'],
                 dict(to_revision='7b2661a43ba3', from_revision=None, show_sql_only=True),
             ),
             (
-                ['--revision', 'abc', '--from-revision', 'abc123', '--show-sql-only'],
+                ['--to-revision', 'abc', '--from-revision', 'abc123', '--show-sql-only'],
                 dict(to_revision='abc', from_revision='abc123', show_sql_only=True),
             ),
             (
-                ['--revision', 'abc', '--from-version', '2.2.2', '--show-sql-only'],
+                ['--to-revision', 'abc', '--from-version', '2.2.2', '--show-sql-only'],
                 dict(to_revision='abc', from_revision='7b2661a43ba3', show_sql_only=True),
             ),
             (
-                ['--version', '2.2.4', '--from-revision', 'abc123', '--show-sql-only'],
+                ['--to-version', '2.2.4', '--from-revision', 'abc123', '--show-sql-only'],
                 dict(to_revision='587bdf053233', from_revision='abc123', show_sql_only=True),
             ),
             (
-                ['--version', '2.2.4', '--from-version', '2.2.2', '--show-sql-only'],
+                ['--to-version', '2.2.4', '--from-version', '2.2.2', '--show-sql-only'],
                 dict(to_revision='587bdf053233', from_revision='7b2661a43ba3', show_sql_only=True),
             ),
         ],
@@ -95,19 +95,19 @@ def test_cli_upgrade_success(self, mock_upgradedb, args, called_with):
     @pytest.mark.parametrize(
         'args, pattern',
         [
-            param(['--version', '2.1.25'], 'not supported', id='bad version'),
+            param(['--to-version', '2.1.25'], 'not supported', id='bad version'),
             param(
-                ['--revision', 'abc', '--from-revision', 'abc123'],
+                ['--to-revision', 'abc', '--from-revision', 'abc123'],
                 'used with `--show-sql-only`',
                 id='requires offline',
             ),
             param(
-                ['--revision', 'abc', '--from-version', '2.0.2'],
+                ['--to-revision', 'abc', '--from-version', '2.0.2'],
                 'used with `--show-sql-only`',
                 id='requires offline',
             ),
             param(
-                ['--revision', 'abc', '--from-version', '2.1.25', '--show-sql-only'],
+                ['--to-revision', 'abc', '--from-version', '2.1.25', '--show-sql-only'],
                 'Unknown version',
                 id='bad version',
             ),
@@ -201,15 +201,15 @@ def test_cli_shell_invalid(self):
     @pytest.mark.parametrize(
         'args, match',
         [
-            (['-y', '--revision', 'abc', '--version', '2.2.0'], 'Cannot supply both'),
-            (['-y', '--revision', 'abc1', '--from-revision', 'abc2'], 'only .* with `--show-sql-only`'),
-            (['-y', '--revision', 'abc1', '--from-version', '2.2.2'], 'only .* with `--show-sql-only`'),
-            (['-y', '--version', '2.2.2', '--from-version', '2.2.2'], 'only .* with `--show-sql-only`'),
+            (['-y', '--to-revision', 'abc', '--to-version', '2.2.0'], 'Cannot supply both'),
+            (['-y', '--to-revision', 'abc1', '--from-revision', 'abc2'], 'only .* with `--show-sql-only`'),
+            (['-y', '--to-revision', 'abc1', '--from-version', '2.2.2'], 'only .* with `--show-sql-only`'),
+            (['-y', '--to-version', '2.2.2', '--from-version', '2.2.2'], 'only .* with `--show-sql-only`'),
             (
-                ['-y', '--revision', 'abc', '--from-version', '2.2.0', '--from-revision', 'abc'],
+                ['-y', '--to-revision', 'abc', '--from-version', '2.2.0', '--from-revision', 'abc'],
                 'may not be combined',
             ),
-            (['-y', '--version', 'abc'], r'Downgrading to .* not supported\.'),
+            (['-y', '--to-version', 'abc'], r'Downgrading to .* not supported\.'),
             (['-y'], 'Must provide either'),
         ],
     )
@@ -223,20 +223,20 @@ def test_cli_downgrade_invalid(self, mock_dg, args, match):
     @pytest.mark.parametrize(
         'args, expected',
         [
-            (['-y', '--revision', 'abc1'], dict(to_revision='abc1')),
+            (['-y', '--to-revision', 'abc1'], dict(to_revision='abc1')),
             (
-                ['-y', '--revision', 'abc1', '--from-revision', 'abc2', '-s'],
+                ['-y', '--to-revision', 'abc1', '--from-revision', 'abc2', '-s'],
                 dict(to_revision='abc1', from_revision='abc2', show_sql_only=True),
             ),
             (
-                ['-y', '--revision', 'abc1', '--from-version', '2.2.2', '-s'],
+                ['-y', '--to-revision', 'abc1', '--from-version', '2.2.2', '-s'],
                 dict(to_revision='abc1', from_revision='7b2661a43ba3', show_sql_only=True),
             ),
             (
-                ['-y', '--version', '2.2.2', '--from-version', '2.2.2', '-s'],
+                ['-y', '--to-version', '2.2.2', '--from-version', '2.2.2', '-s'],
                 dict(to_revision='7b2661a43ba3', from_revision='7b2661a43ba3', show_sql_only=True),
             ),
-            (['-y', '--version', '2.2.2'], dict(to_revision='7b2661a43ba3')),
+            (['-y', '--to-version', '2.2.2'], dict(to_revision='7b2661a43ba3')),
         ],
     )
     @mock.patch("airflow.utils.db.downgrade")
@@ -260,9 +260,9 @@ def test_cli_downgrade_confirm(self, mock_input, mock_dg, resp, raise_):
         mock_input.return_value = resp
         if raise_:
             with pytest.raises(SystemExit):
-                db_command.downgrade(self.parser.parse_args(['db', 'downgrade', '--revision', 'abc']))
+                db_command.downgrade(self.parser.parse_args(['db', 'downgrade', '--to-revision', 'abc']))
         else:
-            db_command.downgrade(self.parser.parse_args(['db', 'downgrade', '--revision', 'abc']))
+            db_command.downgrade(self.parser.parse_args(['db', 'downgrade', '--to-revision', 'abc']))
             mock_dg.assert_called_with(to_revision='abc', from_revision=None, show_sql_only=False)