diff --git a/airflow-core/newsfragments/68354.bugfix.rst b/airflow-core/newsfragments/68354.bugfix.rst new file mode 100644 index 0000000000000..ba5c01f4c5a8e --- /dev/null +++ b/airflow-core/newsfragments/68354.bugfix.rst @@ -0,0 +1 @@ +Fix FAB roles and users admin pages returning 500 when connection hook metadata is loaded concurrently diff --git a/airflow-core/src/airflow/api_fastapi/core_api/services/ui/connections.py b/airflow-core/src/airflow/api_fastapi/core_api/services/ui/connections.py index 12e823ce1d803..cd03cb895401a 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/services/ui/connections.py +++ b/airflow-core/src/airflow/api_fastapi/core_api/services/ui/connections.py @@ -27,10 +27,11 @@ ConnectionHookMetaData, StandardHookFields, ) +from airflow.providers_manager import HookInfo, ProvidersManager from airflow.serialization.definitions.param import SerializedParam if TYPE_CHECKING: - from airflow.providers_manager import ConnectionFormWidgetInfo, HookInfo + from airflow.providers_manager import ConnectionFormWidgetInfo log = logging.getLogger(__name__) @@ -126,8 +127,6 @@ def _get_hooks_with_mocked_fab() -> tuple[ """Get hooks with all details w/o FAB needing to be installed.""" from unittest import mock - from airflow.providers_manager import ProvidersManager - def mock_lazy_gettext(txt: str) -> str: """Mock for flask_babel.lazy_gettext.""" return txt @@ -158,12 +157,7 @@ def mock_any_of(allowed_values: list) -> HookMetaService.MockEnum: except ModuleNotFoundError: sys.modules[mod_name] = MagicMock() - # We conditionally inject mock classes for missing dependencies - # to ensure `ProvidersManager` can initialize hook connection widgets - # without crashing when FAB/WTForms are not installed. - if "wtforms.StringField" not in sys.modules: - # Only apply mocks if the actual module wasn't loaded beforehand. - # This avoids thread-safety issues caused by `unittest.mock.patch` mutating global states. + if "wtforms" not in sys.modules: with ( mock.patch("wtforms.StringField", HookMetaService.MockStringField), mock.patch("wtforms.fields.StringField", HookMetaService.MockStringField), @@ -282,19 +276,17 @@ def _convert_extra_fields(form_widgets: dict[str, ConnectionFormWidgetInfo]) -> @staticmethod @cache def hook_meta_data() -> list[ConnectionHookMetaData]: - hooks, connection_form_widgets, field_behaviours = HookMetaService._get_hooks_with_mocked_fab() - result: list[ConnectionHookMetaData] = [] - widgets = HookMetaService._convert_extra_fields(connection_form_widgets) - for hook_key, hook_info in hooks.items(): - if not hook_info: - continue - hook_meta = ConnectionHookMetaData( + pm = ProvidersManager() + hook_items = [(hook_key, hook_info) for hook_key, hook_info in pm.hooks.items() if hook_info] + widgets = HookMetaService._convert_extra_fields(pm._connection_form_widgets) + return [ + ConnectionHookMetaData( connection_type=hook_key, hook_class_name=hook_info.hook_class_name, - default_conn_name=None, # TODO: later + default_conn_name=None, hook_name=hook_info.hook_name, - standard_fields=HookMetaService._make_standard_fields(field_behaviours.get(hook_key)), + standard_fields=HookMetaService._make_standard_fields(pm._field_behaviours.get(hook_key)), extra_fields=widgets.get(hook_key), ) - result.append(hook_meta) - return result + for hook_key, hook_info in hook_items + ] diff --git a/airflow-core/tests/unit/api_fastapi/core_api/services/ui/test_connections.py b/airflow-core/tests/unit/api_fastapi/core_api/services/ui/test_connections.py index e6ed84778d31b..6ff1536ac3da5 100644 --- a/airflow-core/tests/unit/api_fastapi/core_api/services/ui/test_connections.py +++ b/airflow-core/tests/unit/api_fastapi/core_api/services/ui/test_connections.py @@ -18,6 +18,8 @@ from airflow.api_fastapi.core_api.services.ui.connections import HookMetaService +from tests_common.test_utils.markers import skip_if_force_lowest_dependencies_marker + class TestMockOptional: def test_mock_optional_is_callable(self): @@ -30,3 +32,28 @@ def test_mock_optional_call_is_noop(self): validator = HookMetaService.MockOptional() result = validator(None, None) assert result is None + + +class TestHookMetaServiceFabWidgetSafety: + @skip_if_force_lowest_dependencies_marker + def test_hook_meta_data_does_not_patch_fab_widgets(self): + import wtforms # noqa: F401 + from flask_appbuilder.fieldwidgets import BS3TextFieldWidget as widget_before + + HookMetaService.hook_meta_data.cache_clear() + HookMetaService.hook_meta_data() + + from flask_appbuilder.fieldwidgets import BS3TextFieldWidget as widget_after + + assert widget_before is widget_after + + @skip_if_force_lowest_dependencies_marker + def test_get_hooks_with_mocked_fab_skips_mocks_when_wtforms_loaded(self): + import wtforms # noqa: F401 + from flask_appbuilder.fieldwidgets import BS3TextFieldWidget as widget_before + + HookMetaService._get_hooks_with_mocked_fab() + + from flask_appbuilder.fieldwidgets import BS3TextFieldWidget as widget_after + + assert widget_before is widget_after