Merge pull request #1303 from gevent/drop-path-import-support

Drop the deprecated ability to use absolute paths when giving an importable setting

CHANGES.rst

@@ -31,6 +31,11 @@
 - Improve safety of handling exceptions during interpreter shutdown.
   See :issue:`1295` reported by BobDenar1212.
 
+- Remove the deprecated ability to specify ``GEVENT_RESOLVER`` and
+  other importable settings as a ``path/to/a/package.module.item``.
+  This had race conditions and didn't work with complicated resolver
+  implementations. Place the required package or module on `sys.path`
+  first.
 
 1.3.7 (2018-10-12)
 ==================

src/gevent/_config.py

@@ -12,7 +12,6 @@ from __future__ import print_function, absolute_import, division
 
 import importlib
 import os
-import sys
 import textwrap
 
 from gevent._compat import string_types
@@ -210,78 +209,60 @@ class Config(object):
 
 class ImportableSetting(object):
 
-    def _import(self, path, _NONE=object):
-        # pylint:disable=too-many-branches
-        if isinstance(path, list):
-            if not path:
-                raise ImportError('Cannot import from empty list: %r' % (path, ))
+    def _import_one_of(self, candidates):
+        assert isinstance(candidates, list)
+        if not candidates:
+            raise ImportError('Cannot import from empty list')
 
-            for item in path[:-1]:
-                try:
-                    return self._import(item)
-                except ImportError:
-                    pass
+        for item in candidates[:-1]:
+            try:
+                return self._import_one(item)
+            except ImportError:
+                pass
 
-            return self._import(path[-1])
+        return self._import_one(candidates[-1])
 
+    def _import_one(self, path, _MISSING=object()):
         if not isinstance(path, string_types):
             return path
 
-        if '.' not in path:
+        if '.' not in path or '/' in path:
             raise ImportError("Cannot import %r. "
-                              "Required format: [path/][package.]module.class. "
+                              "Required format: [package.]module.class. "
                               "Or choose from %r"
                               % (path, list(self.shortname_map)))
 
-        if '/' in path:
-            # This is dangerous, subject to race conditions, and
-            # may not work properly for things like namespace packages
-            import warnings
-            warnings.warn("Absolute paths are deprecated and will be removed in 1.4."
-                          "Please put the package on sys.path first",
-                          DeprecationWarning)
-            package_path, path = path.rsplit('/', 1)
-            sys.path = [package_path] + sys.path
-        else:
-            package_path = None
-
-        try:
-            module, item = path.rsplit('.', 1)
-            module = importlib.import_module(module)
-            x = getattr(module, item, _NONE)
-            if x is _NONE:
-                raise ImportError('Cannot import %r from %r' % (item, module))
-            return x
-        finally:
-            if package_path:
-                try:
-                    sys.path.remove(package_path)
-                except ValueError: # pragma: no cover
-                    pass
+
+        module, item = path.rsplit('.', 1)
+        module = importlib.import_module(module)
+        x = getattr(module, item, _MISSING)
+        if x is _MISSING:
+            raise ImportError('Cannot import %r from %r' % (item, module))
+        return x
 
     shortname_map = {}
 
     def validate(self, value):
         if isinstance(value, type):
             return value
-        return self._import([self.shortname_map.get(x, x) for x in value])
+        return self._import_one_of([self.shortname_map.get(x, x) for x in value])
 
     def get_options(self):
         result = {}
         for name, val in self.shortname_map.items():
             try:
-                result[name] = self._import(val)
+                result[name] = self._import_one(val)
             except ImportError as e:
-                import traceback
-                traceback.print_exc()
                 result[name] = e
         return result
 
+
 class BoolSettingMixin(object):
     validate = staticmethod(validate_bool)
     # Don't do string-to-list conversion.
     _convert = staticmethod(convert_str_value_as_is)
 
+
 class IntSettingMixin(object):
     # Don't do string-to-list conversion.
     def _convert(self, value):

src/greentest/test___config.py

@@ -120,28 +120,35 @@ class TestImportableSetting(unittest.TestCase):
         i = _config.ImportableSetting()
         with self.assertRaisesRegex(ImportError,
                                     "Cannot import from empty list"):
-            i._import([])
+            i._import_one_of([])
 
-    def test_path(self):
+    def test_path_not_supported(self):
         import warnings
         i = _config.ImportableSetting()
         path = list(sys.path)
         with warnings.catch_warnings(record=True) as w:
             warnings.simplefilter("always")
             with self.assertRaisesRegex(ImportError,
-                                        "Cannot import 'no_such_module'"):
-                i._import('foo/bar/gevent.no_such_module')
+                                        "Cannot import 'foo/bar/gevent.no_such_module'"):
+                i._import_one('foo/bar/gevent.no_such_module')
 
         # We restored the path
         self.assertEqual(path, sys.path)
 
-        self.assertEqual(len(w), 1)
-        self.assertEqual(w[0].category, DeprecationWarning)
-        self.assertIn('Absolute paths', str(w[0].message))
+        # We did not issue a warning
+        self.assertEqual(len(w), 0)
 
     def test_non_string(self):
         i = _config.ImportableSetting()
-        self.assertIs(i._import(self), self)
+        self.assertIs(i._import_one(self), self)
+
+    def test_get_options(self):
+        i = _config.ImportableSetting()
+        self.assertEqual({}, i.get_options())
+
+        i.shortname_map = {'foo': 'bad/path'}
+        options = i.get_options()
+        self.assertIn('foo', options)
 
 if __name__ == '__main__':
     unittest.main()