pierrejeambrun commented on code in PR #42019:
URL: https://github.com/apache/airflow/pull/42019#discussion_r1745464971
##########
airflow/api_ui/app.py:
##########
@@ -32,17 +38,35 @@ def init_dag_bag(app: FastAPI) -> None:
app.state.dag_bag = get_dag_bag()
-def create_app() -> FastAPI:
+def init_flask_app(app: FastAPI, testing: bool = False) -> None:
+ """
+ Auth providers and permission logic are tightly coupled to Flask.
Review Comment:
Yes indeed we really do not want to have Flask around anymore I believe when
we release airflow 3. There is no reason to if all of our APIs are on FastAPI.
Yes FastAPI has async support natively, but we cannot have that 'freely' now
because most of the db utility code of airflow, for instance how to create
sessions and provide them via a decorator, how to create the db engine, context
managers etc are not yet written in an async way, so we would need to write
that. (I think that would be part of AIP-70). It is definitely the endgoal to
be fully async for a FastAPI. Maybe we don't need AIP-70 for that but it's a
significant effort to rewrite all of those and not critical for the POC to
work. Also regarding the migration of endpionts from the old api to the new api
it will be a bit easier if both are sync, basically that's close to a copy and
past (i am exaggerating a bit but you get the idea). More code will need to be
adapted if the new endpoints are async while the old ones are not. Maybe it is
better to do that in a 'second' step, when everything is working in fastapi
synchronously.
> do not think there is (maybe due to compatibility but can be removed) any
requirement to use flask in our Auth Manager?
> (Maybe I do not understand the tight-coupling here) - but I don't think we
are in any way tightly coupled with flask (other than compatibility
requirements for the old plugins).
Those are the problem at the moment I believe. At multiple places in our
backend_auth, permission code, etc.. etc..., we use the flask app context to
retrieve global things such as the connected user, the sessions, the current
request. Those are not available when running FastAPI, to give some exemples:
- `FABAuthManager` uses flask_login.current_user, connexion.FlaskAPI,
- Almost all the backend_auth accessing the flask app or app context
- `FabAirflowSecurityManagerOverride` has calls to flask libraries to handle
jwt or openid or access directly the flask `g` global object to retrieve the
user. Uses `flask_login.LoginManager`, `flask.flash` or the `session` global
object.
- The `AnonymousUser` is using the flask.current_app() and a `flask_login`
mixin.
- Most of the places where an AirflowApp, AppBuilder is required, we need a
flask app underneath
Those are just some exemples, there a plenty more in the code, maybe I am
misunderstanding something and Vincent/Jed will have more information on that
and we can simply re-write another SecurityManager / AuthManager agnostic of
flask things will be ok.
Also just to mention that FastAPI and Flask work in two different paradigm.
Flask uses global object and application context accessor basically via
importing `g` or `current_user` and using them where you need them in the views
/ provider / utility / managers. In opposition FastAPI uses dependency
injection, where function signature declares its dependency and FastAPI will
inject them at runtime. There might be some refactoring involved due to this
shift.
--
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]
