potiuk commented on code in PR #36537:
URL: https://github.com/apache/airflow/pull/36537#discussion_r1443779310


##########
airflow/providers/fab/provider.yaml:
##########
@@ -36,6 +36,11 @@ versions:
 dependencies:
   - apache-airflow>=2.9.0
   - flask>=2.2,<2.3
+  # We are tightly coupled with FAB version as we vendored-in part of FAB code 
related to security manager
+  # This is done as part of preparation to removing FAB as dependency, but we 
are not ready for it yet
+  # Every time we update FAB version here, please make sure that you review 
the classes and models in
+  # `airflow/providers/fan/auth_manager/security_manager/override.py` with 
their upstream counterparts.

Review Comment:
   There are good reasons (historical) of course. And the reason is that 
originally FAB security manager does a bit too much - namely it also exposes 
FAB-version of user management (which we implemented on our own and this is 
what is available in Airflow UI.  But yeah. That's a topic for a different 
discussion. And actually with FAB provider separated, it **could** be possible 
to get rid of that partial copying - though it might be rather complex thing to 
do.
   
   BTW. I was not the one who decided on it, but if  I was around that I'd also 
strongly oppose that decision and try to find another solution - what we do now 
is borderline crazy.



-- 
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]

Reply via email to