Make auth managers provide their own airflow CLI commands #33481
Conversation
| @@ -182,7 +182,7 @@ def initialize_database(self): | |||
| # server. Thus, we make a random password and store it in AIRFLOW_HOME, | |||
| # with the reasoning that if you can read that directory, you can see | |||
| # the database credentials anyway. | |||
| from airflow.utils.cli_app_builder import get_application_builder | |||
| from airflow.auth.managers.fab.cli_commands.utils import get_application_builder | |||
There was a problem hiding this comment.
This is not good. We do not want Airflow to have a dependency on FAB auth manager. People might use another Auth manager and not even have FAB auth manager installed (this is assuming FAB auth manager is moved to a separate provider)
There was a problem hiding this comment.
yes I know, we talked a bit about it on Thursday when we looked at the code together :)
I moved the "easy" commands in the PR, this one will need to move (at least partially) too, but the situation is a little bit more complex, I suggest dealing with it in a separate PR, where I'd introduce an init_airflow method in auth managers for instance.
There was a problem hiding this comment.
I am okay with that but we definitely do not want to forget it. This is something we really dont want
|
Does it resolve #32348 as well? |
ah yes indeed, I didn't see there was a separate ticket for this |
vandonr-amz
requested review from
ryanahamilton,
ashb,
bbovenzi and
pierrejeambrun
as code owners
|
@vincbeck you left a "requesting changes" review, I think your concerns have been addressed ? |
Approved |
ephraimbuddy
added
the
changelog:skip
closes #32206
closes #32208
closes #32348
taking a lot of inspiration from #29055
Since auth managers are going to be configurable, we don't want their specific commands to be defined in airflow core.
Thus, this PR moves (almost) everything auth manager-related to the fab corner, and adds a mechanism so that auth managers can provide their own CLI commands.
On my computer, running this with the FAB auth manager makes the commands take 10 to 15 extra milliseconds, which seems reasonable.