vincbeck commented on code in PR #45009:
URL: https://github.com/apache/airflow/pull/45009#discussion_r1892469888
##########
airflow/auth/managers/base_auth_manager.py:
##########
@@ -68,14 +68,8 @@ class BaseAuthManager(Generic[T], LoggingMixin):
Class to derive in order to implement concrete auth managers.
Auth managers are responsible for any user management related operation
such as login, logout, authz, ...
-
- :param appbuilder: the flask app builder
"""
- def __init__(self, appbuilder: AirflowAppBuilder | None = None) -> None:
Review Comment:
You are correct
##########
providers/src/airflow/providers/amazon/aws/auth_manager/aws_auth_manager.py:
##########
@@ -76,12 +76,12 @@ class AwsAuthManager(BaseAuthManager):
Leverages AWS services such as Amazon Identity Center and Amazon Verified
Permissions to perform
authentication and authorization in Airflow.
-
- :param appbuilder: the flask app builder
"""
- def __init__(self, appbuilder: AirflowAppBuilder) -> None:
- super().__init__(appbuilder)
+ appbuilder: AirflowAppBuilder | None = None
+
+ def __init__(self) -> None:
+ super().__init__()
Review Comment:
Yes, I'll make the AWS auth manager compatible only with Airflow 3. I dont
think many users use it anyway.
##########
airflow/auth/managers/simple/simple_auth_manager.py:
##########
@@ -80,6 +81,9 @@ class
SimpleAuthManager(BaseAuthManager[SimpleAuthManagerUser]):
# Cache containing the password associated to a username
passwords: dict[str, str] = {}
+ # TODO: Needs to be deleted when Airflow 2 legacy UI is gone
+ appbuilder: AirflowAppBuilder | None = None
Review Comment:
I agree but at the same time the auth managers, today, need FAB and I
actually like having it declare in all the auth managers so everyone is aware.
All auth managers need to create custom views for their own use. Example: the
simple auth manager define its own login form. It adds this view to the
environment using FAB. Today we do not have mechanism to add views to Airflow
3. I was counting on AIP-68 but it seems it is kind of blocked.
I added this one initially for typing and I am pretty sure we could remove
it and work it out but again, I kinda like it is there so that we know that all
auth managers need FAB.
--
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]
