Skip to content

Commit 4ec625c

Browse files
ashbChris Fei
authored and
Chris Fei
committed
Replace pkg_resources with importlib.metadata to avoid VersionConflict errors (apache#12694)
Using `pkg_resources.iter_entry_points` validates the version constraints, and if any fail it will throw an Exception for that entrypoint. This sounds nice, but is a huge mis-feature. So instead of that, switch to using importlib.metadata (well, it's backport importlib_metadata) that just gives us the entrypoints - no other verification of requirements is performed. This has two advantages: 1. providers and plugins load much more reliably. 2. it's faster too Closes apache#12692 (cherry picked from commit 7ef9aa7)
1 parent e602ae5 commit 4ec625c

File tree

3 files changed

+72
-40
lines changed

3 files changed

+72
-40
lines changed

airflow/plugins_manager.py

+31-8
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,13 @@
3232
import warnings
3333
from typing import Any, Dict, List, Type
3434

35-
import pkg_resources
3635
from six import with_metaclass
3736

37+
try:
38+
import importlib.metadata as importlib_metadata
39+
except ImportError:
40+
import importlib_metadata
41+
3842
from airflow import settings
3943
from airflow.models.baseoperator import BaseOperatorLink
4044

@@ -109,6 +113,23 @@ def on_load(cls, *args, **kwargs):
109113
"""
110114

111115

116+
def entry_points_with_dist(group):
117+
"""
118+
Return EntryPoint objects of the given group, along with the distribution information.
119+
120+
This is like the ``entry_points()`` function from importlib.metadata,
121+
except it also returns the distribution the entry_point was loaded from.
122+
123+
:param group: FIlter results to only this entrypoint group
124+
:return: Generator of (EntryPoint, Distribution) objects for the specified groups
125+
"""
126+
for dist in importlib_metadata.distributions():
127+
for e in dist.entry_points:
128+
if e.group != group:
129+
continue
130+
yield (e, dist)
131+
132+
112133
def load_entrypoint_plugins(entry_points, airflow_plugins):
113134
"""
114135
Load AirflowPlugin subclasses from the entrypoints
@@ -122,16 +143,18 @@ def load_entrypoint_plugins(entry_points, airflow_plugins):
122143
:rtype: list[airflow.plugins_manager.AirflowPlugin]
123144
"""
124145
global import_errors # pylint: disable=global-statement
125-
for entry_point in entry_points:
146+
for entry_point, dist in entry_points:
126147
log.debug('Importing entry_point plugin %s', entry_point.name)
127148
try:
128149
plugin_obj = entry_point.load()
129-
plugin_obj.__usable_import_name = entry_point.module_name
130-
if is_valid_plugin(plugin_obj, airflow_plugins):
131-
if callable(getattr(plugin_obj, 'on_load', None)):
132-
plugin_obj.on_load()
150+
plugin_obj.__usable_import_name = entry_point.module
151+
if not is_valid_plugin(plugin_obj, airflow_plugins):
152+
continue
153+
154+
if callable(getattr(plugin_obj, 'on_load', None)):
155+
plugin_obj.on_load()
133156

134-
airflow_plugins.append(plugin_obj)
157+
airflow_plugins.append(plugin_obj)
135158
except Exception as e: # pylint: disable=broad-except
136159
log.exception("Failed to import plugin %s", entry_point.name)
137160
import_errors[entry_point.module_name] = str(e)
@@ -204,7 +227,7 @@ def is_valid_plugin(plugin_obj, existing_plugins):
204227
import_errors[filepath] = str(e)
205228

206229
plugins = load_entrypoint_plugins(
207-
pkg_resources.iter_entry_points('airflow.plugins'),
230+
entry_points_with_dist('airflow.plugins'),
208231
plugins
209232
)
210233

tests/plugins/test_plugins_manager_rbac.py

+30-26
Original file line numberDiff line numberDiff line change
@@ -22,17 +22,16 @@
2222
from __future__ import print_function
2323
from __future__ import unicode_literals
2424

25-
import unittest
26-
import six
27-
from tests.compat import mock
25+
import logging
2826

29-
import pkg_resources
27+
import pytest
3028

3129
from airflow.www_rbac import app as application
30+
from tests.compat import mock
3231

3332

34-
class PluginsTestRBAC(unittest.TestCase):
35-
def setUp(self):
33+
class TestPluginsRBAC(object):
34+
def setup_method(self, method):
3635
self.app, self.appbuilder = application.create_app(testing=True)
3736

3837
def test_flaskappbuilder_views(self):
@@ -41,18 +40,18 @@ def test_flaskappbuilder_views(self):
4140
plugin_views = [view for view in self.appbuilder.baseviews
4241
if view.blueprint.name == appbuilder_class_name]
4342

44-
self.assertTrue(len(plugin_views) == 1)
43+
assert len(plugin_views) == 1
4544

4645
# view should have a menu item matching category of v_appbuilder_package
4746
links = [menu_item for menu_item in self.appbuilder.menu.menu
4847
if menu_item.name == v_appbuilder_package['category']]
4948

50-
self.assertTrue(len(links) == 1)
49+
assert len(links) == 1
5150

5251
# menu link should also have a link matching the name of the package.
5352
link = links[0]
54-
self.assertEqual(link.name, v_appbuilder_package['category'])
55-
self.assertEqual(link.childs[0].name, v_appbuilder_package['name'])
53+
assert link.name == v_appbuilder_package['category']
54+
assert link.childs[0].name == v_appbuilder_package['name']
5655

5756
def test_flaskappbuilder_menu_links(self):
5857
from tests.plugins.test_plugin import appbuilder_mitem
@@ -61,40 +60,45 @@ def test_flaskappbuilder_menu_links(self):
6160
links = [menu_item for menu_item in self.appbuilder.menu.menu
6261
if menu_item.name == appbuilder_mitem['category']]
6362

64-
self.assertTrue(len(links) == 1)
63+
assert len(links) == 1
6564

6665
# menu link should also have a link matching the name of the package.
6766
link = links[0]
68-
self.assertEqual(link.name, appbuilder_mitem['category'])
69-
self.assertEqual(link.childs[0].name, appbuilder_mitem['name'])
67+
assert link.name == appbuilder_mitem['category']
68+
assert link.childs[0].name == appbuilder_mitem['name']
7069

7170
def test_app_blueprints(self):
7271
from tests.plugins.test_plugin import bp
7372

7473
# Blueprint should be present in the app
75-
self.assertTrue('test_plugin' in self.app.blueprints)
76-
self.assertEqual(self.app.blueprints['test_plugin'].name, bp.name)
74+
assert 'test_plugin' in self.app.blueprints
75+
assert self.app.blueprints['test_plugin'].name == bp.name
7776

78-
@unittest.skipIf(six.PY2, 'self.assertLogs not available for Python 2')
79-
@mock.patch('pkg_resources.iter_entry_points')
80-
def test_entrypoint_plugin_errors_dont_raise_exceptions(self, mock_ep_plugins):
77+
@pytest.mark.quarantined
78+
def test_entrypoint_plugin_errors_dont_raise_exceptions(self, caplog):
8179
"""
8280
Test that Airflow does not raise an Error if there is any Exception because of the
8381
Plugin.
8482
"""
85-
from airflow.plugins_manager import load_entrypoint_plugins, import_errors
83+
from airflow.plugins_manager import import_errors, load_entrypoint_plugins, entry_points_with_dist
84+
85+
mock_dist = mock.Mock()
8686

8787
mock_entrypoint = mock.Mock()
8888
mock_entrypoint.name = 'test-entrypoint'
89+
mock_entrypoint.group = 'airflow.plugins'
8990
mock_entrypoint.module_name = 'test.plugins.test_plugins_manager'
90-
mock_entrypoint.load.side_effect = Exception('Version Conflict')
91-
mock_ep_plugins.return_value = [mock_entrypoint]
91+
mock_entrypoint.load.side_effect = ImportError('my_fake_module not found')
92+
mock_dist.entry_points = [mock_entrypoint]
93+
94+
with mock.patch('importlib_metadata.distributions', return_value=[mock_dist]), caplog.at_level(
95+
logging.ERROR, logger='airflow.plugins_manager'
96+
):
97+
load_entrypoint_plugins(entry_points_with_dist('airflow.plugins'), [])
9298

93-
with self.assertLogs("airflow.plugins_manager", level="ERROR") as log_output:
94-
load_entrypoint_plugins(pkg_resources.iter_entry_points('airflow.plugins'), [])
95-
received_logs = log_output.output[0]
99+
received_logs = caplog.text
96100
# Assert Traceback is shown too
97101
assert "Traceback (most recent call last):" in received_logs
98-
assert "Version Conflict" in received_logs
102+
assert "my_fake_module not found" in received_logs
99103
assert "Failed to import plugin test-entrypoint" in received_logs
100-
assert ('test.plugins.test_plugins_manager', 'Version Conflict') in import_errors.items()
104+
assert ("test.plugins.test_plugins_manager", "my_fake_module not found") in import_errors.items()

tests/plugins/test_plugins_manager_www.py

+11-6
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
from __future__ import unicode_literals
2424

2525
import six
26-
from mock import MagicMock, PropertyMock
26+
from mock import MagicMock, Mock
2727

2828
from flask.blueprints import Blueprint
2929
from flask_admin.menu import MenuLink, MenuView
@@ -119,11 +119,16 @@ def setUp(self):
119119
]
120120

121121
def _build_mock(self, plugin_obj):
122-
m = MagicMock(**{
123-
'load.return_value': plugin_obj
124-
})
125-
type(m).name = PropertyMock(return_value='plugin-' + plugin_obj.name)
126-
return m
122+
123+
mock_dist = Mock()
124+
125+
mock_entrypoint = Mock()
126+
mock_entrypoint.name = 'plugin-' + plugin_obj.name
127+
mock_entrypoint.group = 'airflow.plugins'
128+
mock_entrypoint.load.return_value = plugin_obj
129+
mock_dist.entry_points = [mock_entrypoint]
130+
131+
return (mock_entrypoint, mock_dist)
127132

128133
def test_load_entrypoint_plugins(self):
129134
self.assertListEqual(

0 commit comments

Comments
 (0)