Skip to content

Commit

Permalink
REST API: Fix wrong plugin schema (#34858)
Browse files Browse the repository at this point in the history
* 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.

* fixup! REST API: Fix wrong plugin schema

* Add test at the endpoint

* fixup! Add test at the endpoint
  • Loading branch information
ephraimbuddy committed Oct 12, 2023
1 parent 946b539 commit 474fa4d
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 43 deletions.
8 changes: 4 additions & 4 deletions airflow/api_connexion/openapi/v1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3776,13 +3776,13 @@ components:
macros:
type: array
items:
type: object
type: string
nullable: true
description: The plugin macros
flask_blueprints:
type: array
items:
type: object
type: string
nullable: true
description: The flask blueprints
appbuilder_views:
Expand All @@ -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:
Expand Down
8 changes: 4 additions & 4 deletions airflow/api_connexion/schemas/plugin_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()


Expand Down
8 changes: 4 additions & 4 deletions airflow/www/static/js/types/api-generated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1575,17 +1575,17 @@ 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?: ({ [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;
};
Expand Down
64 changes: 56 additions & 8 deletions tests/api_connexion/endpoints/test_plugin_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,60 @@
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 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


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):
...


mockview = MockView()

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
Expand Down Expand Up @@ -54,22 +100,24 @@ 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"})
assert response.status_code == 200
assert response.json == {
"plugins": [
{
"appbuilder_menu_items": [],
"appbuilder_views": [],
"appbuilder_menu_items": [appbuilder_menu_items],
"appbuilder_views": [{"view": qualname(MockView)}],
"executors": [],
"flask_blueprints": [],
"global_operator_extra_links": [],
"hooks": [],
"macros": [],
"operator_extra_links": [],
"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",
}
Expand Down
90 changes: 67 additions & 23 deletions tests/api_connexion/schemas/test_plugin_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,35 +16,79 @@
# 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"


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",
}
Expand All @@ -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",
},
Expand Down

0 comments on commit 474fa4d

Please sign in to comment.