You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/01/06 16:06:55 UTC

[GitHub] [airflow] feluelle commented on a change in pull request #6696: [AIRFLOW-6128] Simplify plugins_manager and webserver plugin code

feluelle commented on a change in pull request #6696: [AIRFLOW-6128] Simplify plugins_manager and webserver plugin code
URL: https://github.com/apache/airflow/pull/6696#discussion_r363359502
 
 

 ##########
 File path: airflow/www/app.py
 ##########
 @@ -89,167 +176,116 @@ def create_app(config=None, session=None, testing=False, app_name="Airflow"):
     configure_manifest_files(app)
 
     with app.app_context():
+        @app.context_processor
+        def jinja_globals() -> Dict[str, str]:  # pylint: disable=unused-variable
+            """Returns global context variables for JINJA."""
+            global_vars = {
+                'hostname': socket.getfqdn(),
+                'navbar_color': conf.get('webserver', 'NAVBAR_COLOR'),
+            }
+
+            if 'analytics_tool' in conf.getsection('webserver'):
+                global_vars.update({
+                    'analytics_tool': conf.get('webserver', 'ANALYTICS_TOOL'),
+                    'analytics_id': conf.get('webserver', 'ANALYTICS_ID')
+                })
+
+            return global_vars
+
+        @app.teardown_appcontext
+        def shutdown_session(_):  # pylint: disable=unused-variable
+            """Shuts down the Sqlalchemy session."""
+            settings.Session.remove()
+
         from airflow.www.security import AirflowSecurityManager
-        security_manager_class = app.config.get('SECURITY_MANAGER_CLASS') or \
-            AirflowSecurityManager
 
-        if not issubclass(security_manager_class, AirflowSecurityManager):
-            raise Exception(
-                """Your CUSTOM_SECURITY_MANAGER must now extend AirflowSecurityManager,
-                 not FAB's security manager.""")
+        def get_security_manager() -> Type[AirflowSecurityManager]:
+            """Retrieves security manager from configuration."""
+            security_manager_class = app.config.get('SECURITY_MANAGER_CLASS') or AirflowSecurityManager
 
-        appbuilder = AppBuilder(
+            if not issubclass(security_manager_class, AirflowSecurityManager):
+                raise Exception(
+                    f"Your custom security manager class {security_manager_class} "
+                    "must extend AirflowSecurityManager, not just FAB's security manager.")
+            return security_manager_class
+
+        set_appbuilder(AppBuilder(
             app,
             db.session if not session else session,
-            security_manager_class=security_manager_class,
+            security_manager_class=get_security_manager(),
             base_template='appbuilder/baselayout.html',
             update_perms=conf.getboolean('webserver', 'UPDATE_FAB_PERMS'))
+        )
+
+        if not appbuilder:
+            raise AirflowException("The appbuilder should not be None.")
+
+        add_views()
+        integrate_plugins()
+        init_plugin_blueprints()
+        update_fab_permissions()
+
+        from airflow.www.api.experimental import endpoints
 
-        def init_views(appbuilder):
-            from airflow.www import views
-            # Remove the session from scoped_session registry to avoid
-            # reusing a session with a disconnected connection
-            appbuilder.session.remove()
-            appbuilder.add_view_no_menu(views.Airflow())
-            appbuilder.add_view_no_menu(views.DagModelView())
-            appbuilder.add_view(views.DagRunModelView,
-                                "DAG Runs",
-                                category="Browse",
-                                category_icon="fa-globe")
-            appbuilder.add_view(views.JobModelView,
-                                "Jobs",
-                                category="Browse")
-            appbuilder.add_view(views.LogModelView,
-                                "Logs",
-                                category="Browse")
-            appbuilder.add_view(views.SlaMissModelView,
-                                "SLA Misses",
-                                category="Browse")
-            appbuilder.add_view(views.TaskInstanceModelView,
-                                "Task Instances",
-                                category="Browse")
-            appbuilder.add_view(views.ConfigurationView,
-                                "Configurations",
-                                category="Admin",
-                                category_icon="fa-user")
-            appbuilder.add_view(views.ConnectionModelView,
-                                "Connections",
-                                category="Admin")
-            appbuilder.add_view(views.PoolModelView,
-                                "Pools",
-                                category="Admin")
-            appbuilder.add_view(views.VariableModelView,
-                                "Variables",
-                                category="Admin")
-            appbuilder.add_view(views.XComModelView,
-                                "XComs",
-                                category="Admin")
-
-            if "dev" in version.version:
-                airflow_doc_site = "https://airflow.readthedocs.io/en/latest"
-            else:
-                airflow_doc_site = 'https://airflow.apache.org/docs/{}'.format(version.version)
-
-            appbuilder.add_link("Documentation",
-                                href=airflow_doc_site,
-                                category="Docs",
-                                category_icon="fa-cube")
-            appbuilder.add_link("GitHub",
-                                href='https://github.com/apache/airflow',
-                                category="Docs")
-            appbuilder.add_view(views.VersionView,
-                                'Version',
-                                category='About',
-                                category_icon='fa-th')
-
-            def integrate_plugins():
-                """Integrate plugins to the context"""
-                from airflow.plugins_manager import (
-                    flask_appbuilder_views, flask_appbuilder_menu_links
-                )
-
-                for v in flask_appbuilder_views:
-                    log.debug("Adding view %s", v["name"])
-                    appbuilder.add_view(v["view"],
-                                        v["name"],
-                                        category=v["category"])
-                for ml in sorted(flask_appbuilder_menu_links, key=lambda x: x["name"]):
-                    log.debug("Adding menu link %s", ml["name"])
-                    appbuilder.add_link(ml["name"],
-                                        href=ml["href"],
-                                        category=ml["category"],
-                                        category_icon=ml["category_icon"])
-
-            integrate_plugins()
-            # Garbage collect old permissions/views after they have been modified.
-            # Otherwise, when the name of a view or menu is changed, the framework
-            # will add the new Views and Menus names to the backend, but will not
-            # delete the old ones.
-
-        def init_plugin_blueprints(app):
-            from airflow.plugins_manager import flask_blueprints
-
-            for bp in flask_blueprints:
-                log.debug("Adding blueprint %s:%s", bp["name"], bp["blueprint"].import_name)
-                app.register_blueprint(bp["blueprint"])
-
-        init_views(appbuilder)
-        init_plugin_blueprints(app)
-
-        if conf.getboolean('webserver', 'UPDATE_FAB_PERMS'):
-            security_manager = appbuilder.sm
-            security_manager.sync_roles()
-
-        from airflow.www.api.experimental import endpoints as e
         # required for testing purposes otherwise the module retains
         # a link to the default_auth
         if app.config['TESTING']:
             import importlib
-            importlib.reload(e)
-
-        app.register_blueprint(e.api_experimental, url_prefix='/api/experimental')
-
-        @app.context_processor
-        def jinja_globals():  # pylint: disable=unused-variable
+            importlib.reload(endpoints)
 
-            globals = {
-                'hostname': socket.getfqdn(),
-                'navbar_color': conf.get('webserver', 'NAVBAR_COLOR'),
-            }
+        app.register_blueprint(endpoints.api_experimental, url_prefix='/api/experimental')
 
-            if 'analytics_tool' in conf.getsection('webserver'):
-                globals.update({
-                    'analytics_tool': conf.get('webserver', 'ANALYTICS_TOOL'),
-                    'analytics_id': conf.get('webserver', 'ANALYTICS_ID')
-                })
+    return app, appbuilder
 
-            return globals
 
-        @app.teardown_appcontext
-        def shutdown_session(exception=None):  # pylint: disable=unused-variable
-            settings.Session.remove()
-
-    return app, appbuilder
+def configure_app(app_name: str, config: Optional[Dict[str, Union[str, bool]]], testing: bool):
+    """Configure the application."""
+    if conf.getboolean('webserver', 'ENABLE_PROXY_FIX'):
+        app.wsgi_app = ProxyFix(  # type: ignore
+            app.wsgi_app,
+            num_proxies=None,
+            x_for=1,
+            x_proto=1,
+            x_host=1,
+            x_port=1,
+            x_prefix=1
+        )
+    app.secret_key = conf.get('webserver', 'SECRET_KEY')
+    app.config.from_pyfile(settings.WEBSERVER_CONFIG, silent=True)
+    app.config['APP_NAME'] = app_name
+    app.config['TESTING'] = testing
+    app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False
+    app.config['SESSION_COOKIE_HTTPONLY'] = True
+    app.config['SESSION_COOKIE_SECURE'] = conf.getboolean('webserver', 'COOKIE_SECURE')
+    app.config['SESSION_COOKIE_SAMESITE'] = conf.get('webserver', 'COOKIE_SAMESITE')
+    if config:
+        app.config.from_mapping(config)
 
 
-def root_app(env, resp):
+def root_app(_, resp: Callable[[bytes, List[Tuple[str, str]]], None]):
+    """Root application function - returns 404 if the path is not found."""
 
 Review comment:
   Same here.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services