You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "uranusjr (via GitHub)" <gi...@apache.org> on 2023/09/01 04:14:24 UTC

[GitHub] [airflow] uranusjr commented on a diff in pull request #33975: Move the try outside the loop when this is possible in Airflow core

uranusjr commented on code in PR #33975:
URL: https://github.com/apache/airflow/pull/33975#discussion_r1312518291


##########
airflow/www/extensions/init_security.py:
##########
@@ -56,14 +56,14 @@ def init_api_experimental_auth(app):
         pass
 
     app.api_auth = []
-    for backend in auth_backends.split(","):
-        try:
+    try:
+        for backend in auth_backends.split(","):
             auth = import_module(backend.strip())
             auth.init_app(app)
             app.api_auth.append(auth)
-        except ImportError as err:
-            log.critical("Cannot import %s for API authentication due to: %s", backend, err)
-            raise AirflowException(err)
+    except ImportError as err:
+        log.critical("Cannot import %s for API authentication due to: %s", backend, err)
+        raise AirflowException(err)

Review Comment:
   While we are at this, I wonder if we should remove this exception wrapping. It doesn’t have any value from what I can tell. Same for the one in `__init__.py`.



##########
airflow/www/extensions/init_security.py:
##########
@@ -56,14 +56,14 @@ def init_api_experimental_auth(app):
         pass
 
     app.api_auth = []
-    for backend in auth_backends.split(","):
-        try:
+    try:
+        for backend in auth_backends.split(","):
             auth = import_module(backend.strip())
             auth.init_app(app)
             app.api_auth.append(auth)
-        except ImportError as err:
-            log.critical("Cannot import %s for API authentication due to: %s", backend, err)
-            raise AirflowException(err)
+    except ImportError as err:
+        log.critical("Cannot import %s for API authentication due to: %s", backend, err)
+        raise AirflowException(err)

Review Comment:
   While we are at this, I wonder if we should remove this exception wrapping. It doesn’t have any value from what I can tell. Same for the one in `__init__.py`.



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org