Fix DeprecationWarning in import_hook. (#26)

* Fix DeprecationWarning in import_hook

For Python >=3.4, using find_loader is deprecated, and importing it
generates a DeprecationWarning. Mirroring wrapt's implementation of a
similar construct in its importer.py (on which this code seems modeled),
we can try to use the new find_spec first,  then the old python 2.7
fallback, as before. Since the newrelic python agent only supports 2.7+
and 3.5+, there is no need to keep support for the deprecated
find_loader at all.

The immediate result of this change is that one less DeprecationWarning
is fired at runtime when using the newrelic python agent. There are
still other DeprecationWarnings originating in this file that are not
fixed by this commit.

Author: sigvef

newrelic/api/import_hook.py

@@ -18,11 +18,6 @@
 
 _logger = logging.getLogger(__name__)
 
-try:
-    from importlib import find_loader
-except ImportError:
-    find_loader = None
-
 _import_hooks = {}
 
 _ok_modules = (
@@ -173,25 +168,14 @@ def find_module(self, fullname, path=None):
         self._skip[fullname] = True
 
         try:
-            # For Python 3 we need to use find_loader() from the new
-            # importlib module.
-
-            if find_loader:
-                loader = find_loader(fullname, path)
-
-                if loader:
-                    return _ImportHookChainedLoader(loader)
-
-            else:
+            try:
+                from importlib.util import find_spec
+                return _ImportHookChainedLoader(find_spec(fullname).loader)
+            except AttributeError:
+                return None
+            except ImportError:
                 __import__(fullname)
-
-                # If we get this far then the module we are
-                # interested in does actually exist and so return
-                # our loader to trigger import hooks and then return
-                # the module.
-
                 return _ImportHookLoader()
-
         finally:
             del self._skip[fullname]
 

tests/agent_unittests/test_import_hook.py

@@ -0,0 +1,43 @@
+from pytest import raises
+
+
+def test_import_hook_finder():
+    """
+    This asserts the behavior of ImportHookFinder.find_module. It behaves
+    differently depending on whether or not the module it is looking for
+    exists, has been registered with an import hook, and across different
+    python versions.
+    """
+    from newrelic.api.import_hook import ImportHookFinder, register_import_hook
+    import sys
+
+    PY2 = sys.version_info[0] == 2
+
+    finder = ImportHookFinder()
+
+    # a dummy hook just to be able to register hooks for modules
+    def hook(*args, **kwargs):
+        pass
+
+    # Finding a module that does not exist and is not registered returns None.
+    module = finder.find_module('some_module_that_does_not_exist')
+    assert module is None
+
+    # Finding a module that does not exist and is registered behaves
+    # differently on python 2 vs python 3.
+    register_import_hook('some_module_that_does_not_exist', hook)
+    if PY2:
+        with raises(ImportError):
+            module = finder.find_module('some_module_that_does_not_exist')
+    else:
+        module = finder.find_module('some_module_that_does_not_exist')
+        assert module is None
+
+    # Finding a module that exists, but is not registered returns None.
+    module = finder.find_module('newrelic')
+    assert module is None
+
+    # Finding a module that exists, and is registered, finds that module.
+    register_import_hook('newrelic', hook)
+    module = finder.find_module('newrelic')
+    assert module is not None