From bcc34cb1edc9a9ca7948da73defd776232e05d9f Mon Sep 17 00:00:00 2001 From: Ephraim Anierobi Date: Wed, 11 Oct 2023 08:39:46 +0100 Subject: [PATCH 1/4] REST API: Fix wrong plugin schema We serialize some plugin's fields as dictionaries leading to errors when accessing the `/plugins` endpoint. Here's the error: ValueError: dictionary update sequence element #0 has length 1; 2 is required The fields are lists of strings and this PR addresses it. --- airflow/api_connexion/openapi/v1.yaml | 6 +++--- airflow/api_connexion/schemas/plugin_schema.py | 8 ++++---- airflow/www/static/js/types/api-generated.ts | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index 48c9e5e023725..bec2ebf14613e 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -3782,7 +3782,7 @@ components: flask_blueprints: type: array items: - type: object + type: string nullable: true description: The flask blueprints appbuilder_views: @@ -3800,13 +3800,13 @@ components: global_operator_extra_links: type: array items: - type: object + type: string nullable: true description: The global operator extra links operator_extra_links: type: array items: - type: object + type: string nullable: true description: Operator extra links source: diff --git a/airflow/api_connexion/schemas/plugin_schema.py b/airflow/api_connexion/schemas/plugin_schema.py index 780fef17bf76a..4b62111482075 100644 --- a/airflow/api_connexion/schemas/plugin_schema.py +++ b/airflow/api_connexion/schemas/plugin_schema.py @@ -27,12 +27,12 @@ class PluginSchema(Schema): name = fields.String() hooks = fields.List(fields.String()) executors = fields.List(fields.String()) - macros = fields.List(fields.Dict()) - flask_blueprints = fields.List(fields.Dict()) + macros = fields.List(fields.String()) + flask_blueprints = fields.List(fields.String()) appbuilder_views = fields.List(fields.Dict()) appbuilder_menu_items = fields.List(fields.Dict()) - global_operator_extra_links = fields.List(fields.Dict()) - operator_extra_links = fields.List(fields.Dict()) + global_operator_extra_links = fields.List(fields.String()) + operator_extra_links = fields.List(fields.String()) source = fields.String() diff --git a/airflow/www/static/js/types/api-generated.ts b/airflow/www/static/js/types/api-generated.ts index ec10084385e13..8f0b450016177 100644 --- a/airflow/www/static/js/types/api-generated.ts +++ b/airflow/www/static/js/types/api-generated.ts @@ -1577,15 +1577,15 @@ export interface components { /** @description The plugin macros */ macros?: ({ [key: string]: unknown } | null)[]; /** @description The flask blueprints */ - flask_blueprints?: ({ [key: string]: unknown } | null)[]; + flask_blueprints?: (string | null)[]; /** @description The appuilder views */ appbuilder_views?: ({ [key: string]: unknown } | null)[]; /** @description The Flask Appbuilder menu items */ appbuilder_menu_items?: ({ [key: string]: unknown } | null)[]; /** @description The global operator extra links */ - global_operator_extra_links?: ({ [key: string]: unknown } | null)[]; + global_operator_extra_links?: (string | null)[]; /** @description Operator extra links */ - operator_extra_links?: ({ [key: string]: unknown } | null)[]; + operator_extra_links?: (string | null)[]; /** @description The plugin source */ source?: string | null; }; From ada921538ff9c022cf4c5b23f72317bf881416b4 Mon Sep 17 00:00:00 2001 From: Ephraim Anierobi Date: Wed, 11 Oct 2023 11:17:55 +0100 Subject: [PATCH 2/4] fixup! REST API: Fix wrong plugin schema --- .../schemas/test_plugin_schema.py | 90 ++++++++++++++----- 1 file changed, 67 insertions(+), 23 deletions(-) diff --git a/tests/api_connexion/schemas/test_plugin_schema.py b/tests/api_connexion/schemas/test_plugin_schema.py index 2366fe7ea3e8a..179a318fe5021 100644 --- a/tests/api_connexion/schemas/test_plugin_schema.py +++ b/tests/api_connexion/schemas/test_plugin_schema.py @@ -16,20 +16,64 @@ # under the License. from __future__ import annotations +from flask import Blueprint +from flask_appbuilder import BaseView + from airflow.api_connexion.schemas.plugin_schema import ( PluginCollection, plugin_collection_schema, plugin_schema, ) +from airflow.hooks.base import BaseHook +from airflow.models.baseoperator import BaseOperatorLink from airflow.plugins_manager import AirflowPlugin +class PluginHook(BaseHook): + ... + + +def plugin_macro(): + ... + + +class MockOperatorLink(BaseOperatorLink): + name = "mock_operator_link" + + def get_link(self, operator, *, ti_key) -> str: + return "mock_operator_link" + + +bp = Blueprint("mock_blueprint", __name__, url_prefix="/mock_blueprint") + + +class MockView(BaseView): + ... + + +appbuilder_menu_items = { + "name": "mock_plugin", + "href": "https://example.com", +} + + +class MockPlugin(AirflowPlugin): + name = "mock_plugin" + flask_blueprints = [bp] + appbuilder_views = [{"view": MockView()}] + appbuilder_menu_items = [appbuilder_menu_items] + global_operator_extra_links = [MockOperatorLink()] + operator_extra_links = [MockOperatorLink()] + hooks = [PluginHook] + macros = [plugin_macro] + + class TestPluginBase: def setup_method(self) -> None: - self.mock_plugin = AirflowPlugin() + self.mock_plugin = MockPlugin() self.mock_plugin.name = "test_plugin" - self.mock_plugin_2 = AirflowPlugin() + self.mock_plugin_2 = MockPlugin() self.mock_plugin_2.name = "test_plugin_2" @@ -37,14 +81,14 @@ class TestPluginSchema(TestPluginBase): def test_serialize(self): deserialized_plugin = plugin_schema.dump(self.mock_plugin) assert deserialized_plugin == { - "appbuilder_menu_items": [], - "appbuilder_views": [], + "appbuilder_menu_items": [appbuilder_menu_items], + "appbuilder_views": [{"view": self.mock_plugin.appbuilder_views[0]["view"]}], "executors": [], - "flask_blueprints": [], - "global_operator_extra_links": [], - "hooks": [], - "macros": [], - "operator_extra_links": [], + "flask_blueprints": [str(bp)], + "global_operator_extra_links": [str(MockOperatorLink())], + "hooks": [str(PluginHook)], + "macros": [str(plugin_macro)], + "operator_extra_links": [str(MockOperatorLink())], "source": None, "name": "test_plugin", } @@ -58,26 +102,26 @@ def test_serialize(self): assert deserialized == { "plugins": [ { - "appbuilder_menu_items": [], - "appbuilder_views": [], + "appbuilder_menu_items": [appbuilder_menu_items], + "appbuilder_views": [{"view": self.mock_plugin.appbuilder_views[0]["view"]}], "executors": [], - "flask_blueprints": [], - "global_operator_extra_links": [], - "hooks": [], - "macros": [], - "operator_extra_links": [], + "flask_blueprints": [str(bp)], + "global_operator_extra_links": [str(MockOperatorLink())], + "hooks": [str(PluginHook)], + "macros": [str(plugin_macro)], + "operator_extra_links": [str(MockOperatorLink())], "source": None, "name": "test_plugin", }, { - "appbuilder_menu_items": [], - "appbuilder_views": [], + "appbuilder_menu_items": [appbuilder_menu_items], + "appbuilder_views": [{"view": self.mock_plugin.appbuilder_views[0]["view"]}], "executors": [], - "flask_blueprints": [], - "global_operator_extra_links": [], - "hooks": [], - "macros": [], - "operator_extra_links": [], + "flask_blueprints": [str(bp)], + "global_operator_extra_links": [str(MockOperatorLink())], + "hooks": [str(PluginHook)], + "macros": [str(plugin_macro)], + "operator_extra_links": [str(MockOperatorLink())], "source": None, "name": "test_plugin_2", }, From 4c072087bc99146328292df412f4a96d968104ef Mon Sep 17 00:00:00 2001 From: Ephraim Anierobi Date: Wed, 11 Oct 2023 11:28:29 +0100 Subject: [PATCH 3/4] Add test at the endpoint --- airflow/api_connexion/openapi/v1.yaml | 2 +- airflow/www/static/js/types/api-generated.ts | 2 +- .../endpoints/test_plugin_endpoint.py | 59 ++++++++++++++++--- 3 files changed, 53 insertions(+), 10 deletions(-) diff --git a/airflow/api_connexion/openapi/v1.yaml b/airflow/api_connexion/openapi/v1.yaml index bec2ebf14613e..b3194f3881628 100644 --- a/airflow/api_connexion/openapi/v1.yaml +++ b/airflow/api_connexion/openapi/v1.yaml @@ -3776,7 +3776,7 @@ components: macros: type: array items: - type: object + type: string nullable: true description: The plugin macros flask_blueprints: diff --git a/airflow/www/static/js/types/api-generated.ts b/airflow/www/static/js/types/api-generated.ts index 8f0b450016177..99e8754c56488 100644 --- a/airflow/www/static/js/types/api-generated.ts +++ b/airflow/www/static/js/types/api-generated.ts @@ -1575,7 +1575,7 @@ export interface components { /** @description The plugin executors */ executors?: (string | null)[]; /** @description The plugin macros */ - macros?: ({ [key: string]: unknown } | null)[]; + macros?: (string | null)[]; /** @description The flask blueprints */ flask_blueprints?: (string | null)[]; /** @description The appuilder views */ diff --git a/tests/api_connexion/endpoints/test_plugin_endpoint.py b/tests/api_connexion/endpoints/test_plugin_endpoint.py index 26a4ea0aed3ee..05b2bba54a1a3 100644 --- a/tests/api_connexion/endpoints/test_plugin_endpoint.py +++ b/tests/api_connexion/endpoints/test_plugin_endpoint.py @@ -17,7 +17,11 @@ from __future__ import annotations import pytest +from flask import Blueprint +from flask_appbuilder import BaseView +from airflow.hooks.base import BaseHook +from airflow.models.baseoperator import BaseOperatorLink from airflow.plugins_manager import AirflowPlugin from airflow.security import permissions from tests.test_utils.api_connexion_utils import assert_401, create_user, delete_user @@ -25,6 +29,45 @@ from tests.test_utils.mock_plugins import mock_plugin_manager +class PluginHook(BaseHook): + ... + + +def plugin_macro(): + ... + + +class MockOperatorLink(BaseOperatorLink): + name = "mock_operator_link" + + def get_link(self, operator, *, ti_key) -> str: + return "mock_operator_link" + + +bp = Blueprint("mock_blueprint", __name__, url_prefix="/mock_blueprint") + + +class MockView(BaseView): + ... + + +appbuilder_menu_items = { + "name": "mock_plugin", + "href": "https://example.com", +} + + +class MockPlugin(AirflowPlugin): + name = "mock_plugin" + flask_blueprints = [bp] + appbuilder_views = [{"view": MockView()}] + appbuilder_menu_items = [appbuilder_menu_items] + global_operator_extra_links = [MockOperatorLink()] + operator_extra_links = [MockOperatorLink()] + hooks = [PluginHook] + macros = [plugin_macro] + + @pytest.fixture(scope="module") def configured_app(minimal_app_for_api): app = minimal_app_for_api @@ -54,7 +97,7 @@ def setup_attrs(self, configured_app) -> None: class TestGetPlugins(TestPluginsEndpoint): def test_get_plugins_return_200(self): - mock_plugin = AirflowPlugin() + mock_plugin = MockPlugin() mock_plugin.name = "test_plugin" with mock_plugin_manager(plugins=[mock_plugin]): response = self.client.get("api/v1/plugins", environ_overrides={"REMOTE_USER": "test"}) @@ -62,14 +105,14 @@ def test_get_plugins_return_200(self): assert response.json == { "plugins": [ { - "appbuilder_menu_items": [], - "appbuilder_views": [], + "appbuilder_menu_items": [appbuilder_menu_items], + "appbuilder_views": [{"view": mock_plugin.appbuilder_views[0]["view"]}], "executors": [], - "flask_blueprints": [], - "global_operator_extra_links": [], - "hooks": [], - "macros": [], - "operator_extra_links": [], + "flask_blueprints": [str(bp)], + "global_operator_extra_links": [str(MockOperatorLink())], + "hooks": [str(PluginHook)], + "macros": [str(plugin_macro)], + "operator_extra_links": [str(MockOperatorLink())], "source": None, "name": "test_plugin", } From 6925cd86d747a16c8d58bba0590ad4074a35d734 Mon Sep 17 00:00:00 2001 From: Ephraim Anierobi Date: Wed, 11 Oct 2023 14:34:37 +0100 Subject: [PATCH 4/4] fixup! Add test at the endpoint --- .../endpoints/test_plugin_endpoint.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/api_connexion/endpoints/test_plugin_endpoint.py b/tests/api_connexion/endpoints/test_plugin_endpoint.py index 05b2bba54a1a3..a6f67ab5a7017 100644 --- a/tests/api_connexion/endpoints/test_plugin_endpoint.py +++ b/tests/api_connexion/endpoints/test_plugin_endpoint.py @@ -24,6 +24,7 @@ from airflow.models.baseoperator import BaseOperatorLink from airflow.plugins_manager import AirflowPlugin from airflow.security import permissions +from airflow.utils.module_loading import qualname from tests.test_utils.api_connexion_utils import assert_401, create_user, delete_user from tests.test_utils.config import conf_vars from tests.test_utils.mock_plugins import mock_plugin_manager @@ -51,6 +52,8 @@ class MockView(BaseView): ... +mockview = MockView() + appbuilder_menu_items = { "name": "mock_plugin", "href": "https://example.com", @@ -60,7 +63,7 @@ class MockView(BaseView): class MockPlugin(AirflowPlugin): name = "mock_plugin" flask_blueprints = [bp] - appbuilder_views = [{"view": MockView()}] + appbuilder_views = [{"view": mockview}] appbuilder_menu_items = [appbuilder_menu_items] global_operator_extra_links = [MockOperatorLink()] operator_extra_links = [MockOperatorLink()] @@ -106,13 +109,15 @@ def test_get_plugins_return_200(self): "plugins": [ { "appbuilder_menu_items": [appbuilder_menu_items], - "appbuilder_views": [{"view": mock_plugin.appbuilder_views[0]["view"]}], + "appbuilder_views": [{"view": qualname(MockView)}], "executors": [], - "flask_blueprints": [str(bp)], - "global_operator_extra_links": [str(MockOperatorLink())], - "hooks": [str(PluginHook)], - "macros": [str(plugin_macro)], - "operator_extra_links": [str(MockOperatorLink())], + "flask_blueprints": [ + f"<{qualname(bp.__class__)}: name={bp.name!r} import_name={bp.import_name!r}>" + ], + "global_operator_extra_links": [f"<{qualname(MockOperatorLink().__class__)} object>"], + "hooks": [qualname(PluginHook)], + "macros": [qualname(plugin_macro)], + "operator_extra_links": [f"<{qualname(MockOperatorLink().__class__)} object>"], "source": None, "name": "test_plugin", }