Copilot commented on code in PR #64606:
URL: https://github.com/apache/airflow/pull/64606#discussion_r3025324719
##########
airflow-core/src/airflow/plugins_manager.py:
##########
@@ -30,7 +30,13 @@
from airflow._shared.plugins_manager import (
AirflowPlugin,
AirflowPluginSource as AirflowPluginSource,
+ AppBuilderMenuItemDict as AppBuilderMenuItemDict,
+ AppBuilderViewDict as AppBuilderViewDict,
+ ExternalViewDict as ExternalViewDict,
+ FastAPIAppDict as FastAPIAppDict,
+ FastAPIRootMiddlewareDict as FastAPIRootMiddlewareDict,
PluginsDirectorySource as PluginsDirectorySource,
+ ReactAppDict as ReactAppDict,
Review Comment:
The new TypedDict re-exports are imported from
`airflow._shared.plugins_manager`, but that package’s `__init__.py` currently
does not export these names (it only re-exports items from `.plugins_manager`
like `AirflowPlugin`, `make_module`, etc.). As-is, this import will raise
`ImportError` at runtime when `airflow.plugins_manager` is imported.
Suggested fix: re-export the new `*Dict` TypedDicts from
`airflow/_shared/plugins_manager/__init__.py` (and the shared-library
equivalent), or change this import to pull directly from
`airflow._shared.plugins_manager.plugins_manager` where the classes are defined.
##########
shared/plugins_manager/src/airflow_shared/plugins_manager/plugins_manager.py:
##########
@@ -85,6 +85,162 @@ class AirflowPluginException(Exception):
"""Exception when loading plugin."""
+# ---------------------------------------------------------------------------
+# TypedDicts for AirflowPlugin dict-typed attributes
+# ---------------------------------------------------------------------------
+
+
+class FastAPIAppDict(TypedDict):
+ """
+ Dict structure for entries in ``AirflowPlugin.fastapi_apps``.
+
+ Example::
+
+ fastapi_apps = [
+ {
+ "app": my_fastapi_app,
+ "url_prefix": "/my-plugin",
+ "name": "My Plugin",
+ }
+ ]
+ """
+
+ app: Any # FastAPI application instance
+ url_prefix: str
+ name: str
+
+
+class _FastAPIRootMiddlewareDictRequired(TypedDict):
+ middleware: Any # Starlette/ASGI middleware class
+ name: str
+
+
+class FastAPIRootMiddlewareDict(_FastAPIRootMiddlewareDictRequired,
total=False):
+ """
+ Dict structure for entries in ``AirflowPlugin.fastapi_root_middlewares``.
+
+ Example::
+
+ fastapi_root_middlewares = [
+ {
+ "middleware": MyMiddleware,
+ "name": "My Middleware",
+ "args": [],
+ "kwargs": {},
+ }
+ ]
+ """
+
+ args: list[Any]
+ kwargs: dict[str, Any]
+
+
+class _ExternalViewDictRequired(TypedDict):
+ name: str
+ href: str
+
+
+class ExternalViewDict(_ExternalViewDictRequired, total=False):
+ """
+ Dict structure for entries in ``AirflowPlugin.external_views``.
+
+ Example::
+
+ external_views = [
+ {
+ "name": "My Dashboard",
+ "href": "https://dashboard.example.com";,
+ "destination": "nav",
+ "url_route": "my-dashboard",
+ "icon": "https://example.com/icon.svg";,
+ "category": "Tools",
+ }
+ ]
+ """
+
+ icon: str
+ icon_dark_mode: str
+ url_route: str
+ destination: Literal["nav", "dag", "dag_run", "task", "task_instance",
"base"]
+ category: str
+
+
+class _ReactAppDictRequired(TypedDict):
+ name: str
+ bundle_url: str
+
+
+class ReactAppDict(_ReactAppDictRequired, total=False):
+ """
+ Dict structure for entries in ``AirflowPlugin.react_apps``.
+
+ Example::
+
+ react_apps = [
+ {
+ "name": "My React App",
+ "bundle_url": "https://assets.example.com/app.umd.js";,
+ "destination": "nav",
+ "url_route": "my-react-app",
+ "icon": "https://example.com/icon.svg";,
+ "category": "Tools",
+ }
+ ]
+ """
+
+ icon: str
+ icon_dark_mode: str
+ url_route: str
+ destination: Literal["nav", "dag", "dag_run", "task", "task_instance",
"base", "dashboard"]
+ category: str
+
+
+class AppBuilderViewDict(TypedDict, total=False):
+ """
+ Dict structure for entries in ``AirflowPlugin.appbuilder_views``.
+
+ Example::
+
+ appbuilder_views = [
+ {
+ "name": "My View",
+ "category": "My Plugin",
+ "view": my_view_instance,
+ "label": "My View Label",
+ }
+ ]
+ """
+
+ name: str
+ category: str
+ view: Any # Flask-AppBuilder BaseView instance
+ label: str
Review Comment:
`AppBuilderViewDict` is declared with `total=False`, which makes *all* keys
optional (including `view`). However, the FAB integration code assumes
`view["view"]` exists when `name` is not provided (see
`providers/fab/.../init_views.py`), so `view` is effectively required. This
TypedDict will give incorrect guidance to plugin authors.
Suggested fix: use the required/optional split here too (e.g., a required
base TypedDict with `view`, then an optional extension for `name`, `category`,
`label`, etc.).
##########
shared/plugins_manager/src/airflow_shared/plugins_manager/plugins_manager.py:
##########
@@ -85,6 +85,162 @@ class AirflowPluginException(Exception):
"""Exception when loading plugin."""
+# ---------------------------------------------------------------------------
+# TypedDicts for AirflowPlugin dict-typed attributes
+# ---------------------------------------------------------------------------
+
+
+class FastAPIAppDict(TypedDict):
+ """
+ Dict structure for entries in ``AirflowPlugin.fastapi_apps``.
+
+ Example::
+
+ fastapi_apps = [
+ {
+ "app": my_fastapi_app,
+ "url_prefix": "/my-plugin",
+ "name": "My Plugin",
+ }
+ ]
+ """
+
+ app: Any # FastAPI application instance
+ url_prefix: str
+ name: str
+
+
+class _FastAPIRootMiddlewareDictRequired(TypedDict):
+ middleware: Any # Starlette/ASGI middleware class
+ name: str
+
+
+class FastAPIRootMiddlewareDict(_FastAPIRootMiddlewareDictRequired,
total=False):
+ """
+ Dict structure for entries in ``AirflowPlugin.fastapi_root_middlewares``.
+
+ Example::
+
+ fastapi_root_middlewares = [
+ {
+ "middleware": MyMiddleware,
+ "name": "My Middleware",
+ "args": [],
+ "kwargs": {},
+ }
+ ]
+ """
+
+ args: list[Any]
+ kwargs: dict[str, Any]
+
+
+class _ExternalViewDictRequired(TypedDict):
+ name: str
+ href: str
+
+
+class ExternalViewDict(_ExternalViewDictRequired, total=False):
+ """
+ Dict structure for entries in ``AirflowPlugin.external_views``.
+
+ Example::
+
+ external_views = [
+ {
+ "name": "My Dashboard",
+ "href": "https://dashboard.example.com";,
+ "destination": "nav",
+ "url_route": "my-dashboard",
+ "icon": "https://example.com/icon.svg";,
+ "category": "Tools",
+ }
+ ]
+ """
+
+ icon: str
+ icon_dark_mode: str
+ url_route: str
+ destination: Literal["nav", "dag", "dag_run", "task", "task_instance",
"base"]
+ category: str
+
+
+class _ReactAppDictRequired(TypedDict):
+ name: str
+ bundle_url: str
+
+
+class ReactAppDict(_ReactAppDictRequired, total=False):
+ """
+ Dict structure for entries in ``AirflowPlugin.react_apps``.
+
+ Example::
+
+ react_apps = [
+ {
+ "name": "My React App",
+ "bundle_url": "https://assets.example.com/app.umd.js";,
+ "destination": "nav",
+ "url_route": "my-react-app",
+ "icon": "https://example.com/icon.svg";,
+ "category": "Tools",
+ }
+ ]
+ """
+
+ icon: str
+ icon_dark_mode: str
+ url_route: str
+ destination: Literal["nav", "dag", "dag_run", "task", "task_instance",
"base", "dashboard"]
+ category: str
+
+
+class AppBuilderViewDict(TypedDict, total=False):
+ """
+ Dict structure for entries in ``AirflowPlugin.appbuilder_views``.
+
+ Example::
+
+ appbuilder_views = [
+ {
+ "name": "My View",
+ "category": "My Plugin",
+ "view": my_view_instance,
+ "label": "My View Label",
+ }
+ ]
+ """
+
+ name: str
+ category: str
+ view: Any # Flask-AppBuilder BaseView instance
+ label: str
Review Comment:
`AppBuilderViewDict` only models `name`, `category`, `view`, and `label`,
but the FAB integration forwards *all* keys except `view` into
`appbuilder.add_view(..., **filtered_view_kwargs)`. This means valid plugin
dict keys supported by FAB (e.g. `href`, `icon`, `category_icon`,
`category_label`, `menu_cond`, etc.) will be rejected by type checkers.
Suggested fix: expand this TypedDict to include the additional supported FAB
kwargs so the typing matches runtime behavior.
```suggestion
label: str
# Additional kwargs forwarded to ``appbuilder.add_view``.
href: str
icon: str
category_icon: str
category_label: str
menu_cond: Any
```
##########
shared/plugins_manager/src/airflow_shared/plugins_manager/plugins_manager.py:
##########
@@ -85,6 +85,162 @@ class AirflowPluginException(Exception):
"""Exception when loading plugin."""
+# ---------------------------------------------------------------------------
+# TypedDicts for AirflowPlugin dict-typed attributes
+# ---------------------------------------------------------------------------
+
+
+class FastAPIAppDict(TypedDict):
+ """
+ Dict structure for entries in ``AirflowPlugin.fastapi_apps``.
+
+ Example::
+
+ fastapi_apps = [
+ {
+ "app": my_fastapi_app,
+ "url_prefix": "/my-plugin",
+ "name": "My Plugin",
+ }
+ ]
+ """
+
+ app: Any # FastAPI application instance
+ url_prefix: str
+ name: str
+
+
+class _FastAPIRootMiddlewareDictRequired(TypedDict):
+ middleware: Any # Starlette/ASGI middleware class
+ name: str
+
+
+class FastAPIRootMiddlewareDict(_FastAPIRootMiddlewareDictRequired,
total=False):
+ """
+ Dict structure for entries in ``AirflowPlugin.fastapi_root_middlewares``.
+
+ Example::
+
+ fastapi_root_middlewares = [
+ {
+ "middleware": MyMiddleware,
+ "name": "My Middleware",
+ "args": [],
+ "kwargs": {},
+ }
+ ]
+ """
+
+ args: list[Any]
+ kwargs: dict[str, Any]
+
+
+class _ExternalViewDictRequired(TypedDict):
+ name: str
+ href: str
+
+
+class ExternalViewDict(_ExternalViewDictRequired, total=False):
+ """
+ Dict structure for entries in ``AirflowPlugin.external_views``.
+
+ Example::
+
+ external_views = [
+ {
+ "name": "My Dashboard",
+ "href": "https://dashboard.example.com";,
+ "destination": "nav",
+ "url_route": "my-dashboard",
+ "icon": "https://example.com/icon.svg";,
+ "category": "Tools",
+ }
+ ]
+ """
+
+ icon: str
+ icon_dark_mode: str
+ url_route: str
+ destination: Literal["nav", "dag", "dag_run", "task", "task_instance",
"base"]
+ category: str
+
+
+class _ReactAppDictRequired(TypedDict):
+ name: str
+ bundle_url: str
+
+
+class ReactAppDict(_ReactAppDictRequired, total=False):
+ """
+ Dict structure for entries in ``AirflowPlugin.react_apps``.
+
+ Example::
+
+ react_apps = [
+ {
+ "name": "My React App",
+ "bundle_url": "https://assets.example.com/app.umd.js";,
+ "destination": "nav",
+ "url_route": "my-react-app",
+ "icon": "https://example.com/icon.svg";,
+ "category": "Tools",
+ }
+ ]
+ """
+
+ icon: str
+ icon_dark_mode: str
+ url_route: str
+ destination: Literal["nav", "dag", "dag_run", "task", "task_instance",
"base", "dashboard"]
+ category: str
+
+
+class AppBuilderViewDict(TypedDict, total=False):
+ """
+ Dict structure for entries in ``AirflowPlugin.appbuilder_views``.
+
+ Example::
+
+ appbuilder_views = [
+ {
+ "name": "My View",
+ "category": "My Plugin",
+ "view": my_view_instance,
+ "label": "My View Label",
+ }
+ ]
+ """
+
+ name: str
+ category: str
+ view: Any # Flask-AppBuilder BaseView instance
+ label: str
+
+
+class _AppBuilderMenuItemDictRequired(TypedDict):
+ name: str
+ href: str
+
+
+class AppBuilderMenuItemDict(_AppBuilderMenuItemDictRequired, total=False):
+ """
+ Dict structure for entries in ``AirflowPlugin.appbuilder_menu_items``.
+
+ Example::
+
+ appbuilder_menu_items = [
+ {
+ "name": "My Site",
+ "href": "https://example.com";,
+ "category": "Links",
+ }
+ ]
+ """
+
+ category: str
+ label: str
+
Review Comment:
`AppBuilderMenuItemDict` only models `name`, `href`, `category`, and
`label`, but these dicts are passed directly into
`appbuilder.add_link(**menu_link)`, which supports additional kwargs (e.g.
`icon`, `category_icon`, `category_label`, `cond`, etc.). With the current
TypedDict, those valid keys will be rejected by type checkers.
Suggested fix: add the additional supported FAB kwargs to this TypedDict so
it matches the runtime interface.
--
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]
![https://example.com/icon.svg"](https://example.com/icon.svg"){kind=link}