khalidmammadov commented on a change in pull request #20214:
URL: https://github.com/apache/airflow/pull/20214#discussion_r767197252



##########
File path: airflow/www/fab_security/manager.py
##########
@@ -104,12 +106,18 @@ class BaseSecurityManager:
     """ Flask-OpenID OpenID """
     oauth = None
     """ Flask-OAuth """
-    oauth_remotes = None
+    oauth_remotes: Dict[str, Any]
     """ OAuth email whitelists """
-    oauth_whitelists = {}
+    oauth_whitelists: Dict[str, List]
     """ Initialized (remote_app) providers dict {'provider_name', OBJ } """
-    oauth_tokengetter = _oauth_tokengetter
-    """ OAuth tokengetter function override to implement your own tokengetter 
method """
+
+    @staticmethod
+    def oauth_tokengetter():
+        """Authentication (OAuth) token getter function.
+        Override to implement your own token getter method
+        """
+        return _oauth_tokengetter

Review comment:
       > What is the error here? It should be OK to just annotate this as 
`oauth_tokengetter: Any = _oauth_tokengetter` since it is only passed onward to 
Flask-Appbuilder internals.
   
   As far as I remember, it was saying that we are trying to assign a function 
to a class method which does not accept "self".  This then is used to refer at 
below line as a staticmethod:
   
https://github.com/apache/airflow/blob/36aa695ea817ea6f7e446558ef108d2c450ebb0c/airflow/www/fab_security/manager.py#L230
   
   Just tried to reproduce and MyPy saying all ok now without this change. 
Bizarre.. 
   
   But in general, I think it's much clear this way and more explicit. 
Assigning a function to an attribute to make a static method doesn't look OK to 
me. Staticmethod makes is more explicit as per PEP20.  




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