jedcunningham commented on code in PR #50657:
URL: https://github.com/apache/airflow/pull/50657#discussion_r2098299704
##########
airflow-core/src/airflow/cli/cli_config.py:
##########
@@ -1730,7 +1737,59 @@ class GroupCommand(NamedTuple):
),
),
)
-
+DB_MANAGERS_COMMANDS = (
+ ActionCommand(
+ name="reset",
+ help="Burn down and rebuild the metadata database",
Review Comment:
The help docs for these should be rewritten a bit since these are just the
things the db manager controls, not the whole db.
##########
airflow-core/src/airflow/cli/commands/db_manager_command.py:
##########
@@ -18,33 +18,50 @@
from airflow import settings
from airflow.cli.commands.db_command import run_db_downgrade_command,
run_db_migrate_command
-from airflow.providers.fab.auth_manager.models.db import _REVISION_HEADS_MAP,
FABDBManager
+from airflow.configuration import conf
from airflow.utils import cli as cli_utils
+from airflow.utils.module_loading import import_string
from airflow.utils.providers_configuration_loader import
providers_configuration_loaded
+def _get_db_manager(name):
+ """Import the db manager class."""
+ db_managers = conf.get("database", "external_db_managers")
+ db_manager = None
+ for manager in db_managers.split(","):
+ if name in manager:
Review Comment:
Not sure this is right - we want a complete match right, not just a partial
match? If we have a `a` and a `abc` we could match `a` to `abc`. Since we don't
have the concept of a "name", maybe we should take the whole classpath?
##########
airflow-core/src/airflow/cli/commands/db_manager_command.py:
##########
@@ -18,33 +18,50 @@
from airflow import settings
from airflow.cli.commands.db_command import run_db_downgrade_command,
run_db_migrate_command
-from airflow.providers.fab.auth_manager.models.db import _REVISION_HEADS_MAP,
FABDBManager
+from airflow.configuration import conf
from airflow.utils import cli as cli_utils
+from airflow.utils.module_loading import import_string
from airflow.utils.providers_configuration_loader import
providers_configuration_loaded
+def _get_db_manager(name):
+ """Import the db manager class."""
+ db_managers = conf.get("database", "external_db_managers")
+ db_manager = None
+ for manager in db_managers.split(","):
+ if name in manager:
+ db_manager = manager
+ break
+ if db_manager is None:
+ raise SystemExit(f"DB manager {name} not found in configuration.")
+ return import_string(db_manager.strip())
Review Comment:
```suggestion
def _get_db_manager(manager_classpath: str):
"""Import the db manager class."""
db_managers = conf.getlistlist("database", "external_db_managers")
if manager_classpath not in manager:
raise SystemExit(f"DB manager {manager_classpath} not found in
configuration.")
return import_string(manager_classpath.strip())
```
Something like this is probably better.
##########
airflow-core/src/airflow/cli/cli_config.py:
##########
@@ -599,6 +599,13 @@ def string_lower_type(val):
default=False,
)
+ARG_DB_MANAGER_NAME = Arg(
+ ("--name",),
Review Comment:
```suggestion
("name",),
```
Having it be positional might be a bit nicer?
--
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]
