Repository: canonical/operator
Pull request #340 (2020-06-29 16:59:18)

make ops.lib.autoimport use optional
This makes `ops.lib.use` lazy-autoimport, meaning that autoimport is now optional, meaning we don't need to guess whether to use it from main or not.

chipaca: @facundobatista what is the 'change requested' for this PR? A complete refactor of how this works?

Patch:
diff --git a/ops/lib/__init__.py b/ops/lib/__init__.py
index 716cf8c..a4b8041 100644
--- a/ops/lib/__init__.py
+++ b/ops/lib/__init__.py
@@ -21,10 +21,9 @@ from importlib.util import module_from_spec
 from importlib.machinery import ModuleSpec
 from pkgutil import get_importer
 from types import ModuleType
-from typing import Tuple, Dict, List, Iterator, Optional
 
 
-_libraries = {}  # type: Dict[Tuple[str,str], List[_Lib]]
+_libraries = None
 
 _libline_re = re.compile(r'''^LIB([A-Z]+)\s+=\s+([0-9]+|['"][a-zA-Z0-9_.-@]+['"])\s*$''')
 _libname_re = re.compile(r'''^[a-z][a-z0-9]+$''')
@@ -59,6 +58,9 @@ def use(name: str, api: int, author: str) -> ModuleType:
     if not _libauthor_re.match(author):
         raise ValueError("invalid library author email: {!r}".format(author))
 
+    if _libraries is None:
+        autoimport()
+
     versions = _libraries.get((name, author), ())
     for lib in versions:
         if lib.api == api:
@@ -75,7 +77,14 @@ def use(name: str, api: int, author: str) -> ModuleType:
 
 
 def autoimport():
-    _libraries.clear()
+    """Find all libs in the path and enable use of them.
+
+    You only need to call this if you've installed a package or
+    otherwise changed sys.path in the current run, and need to see the
+    changes. Otherwise libraries are found on first call of `use`.
+    """
+    global _libraries
+    _libraries = {}
     for spec in _find_all_specs(sys.path):
         lib = _parse_lib(spec)
         if lib is None:
@@ -86,7 +95,7 @@ def autoimport():
         versions.sort(reverse=True)
 
 
-def _find_all_specs(path: List[str]) -> Iterator[ModuleSpec]:
+def _find_all_specs(path):
     for sys_dir in path:
         if sys_dir == "":
             sys_dir = "."
@@ -117,7 +126,7 @@ def _find_all_specs(path: List[str]) -> Iterator[ModuleSpec]:
 _MAX_LIB_LINES = 99
 
 
-def _parse_lib(spec: ModuleSpec) -> Optional['_Lib']:
+def _parse_lib(spec):
     if spec.origin is None:
         return None
 
@@ -158,7 +167,7 @@ class _Lib:
         self.api = api
         self.patch = patch
 
-        self._module = None  # type: Optional[ModuleType]
+        self._module = None
 
     def __repr__(self):
         return "<_Lib {0.name} by {0.author}, API {0.api}, patch {0.patch}>".format(self)


Test patch:
diff --git a/test/test_lib.py b/test/test_lib.py
index 174293f..b44cfb6 100644
--- a/test/test_lib.py
+++ b/test/test_lib.py
@@ -367,7 +367,7 @@ class TestLibFunctional(TestCase):
         LIBAUTHOR = "alice@example.com"
         """))
 
-        # autoimport would be done in main
+        # autoimport to reset things
         ops.lib.autoimport()
 
         # ops.lib.use done by charm author
@@ -409,7 +409,7 @@ class TestLibFunctional(TestCase):
                         LIBAUTHOR = "alice@example.com"
                         """).format(patch_b))
 
-                        # autoimport would be done in main
+                        # autoimport to reset things
                         ops.lib.autoimport()
 
                         # ops.lib.use done by charm author
@@ -449,7 +449,7 @@ class TestLibFunctional(TestCase):
                         LIBAUTHOR = "alice@example.com"
                         """).format(patch_b))
 
-                        # autoimport would be done in main
+                        # autoimport to reset things
                         ops.lib.autoimport()
 
                         # ops.lib.use done by charm author
@@ -463,6 +463,24 @@ class TestLibFunctional(TestCase):
         with self.assertRaises(ImportError):
             ops.lib.use('foo', 1, 'alice@example.com')
 
+    def test_from_scratch(self):
+        tmpdir = self._mkdtemp()
+        sys.path = [tmpdir]
+
+        _mklib(tmpdir, "foo", "bar").write_text(dedent("""
+        LIBNAME = "baz"
+        LIBAPI = 2
+        LIBPATCH = 42
+        LIBAUTHOR = "alice@example.com"
+        """))
+
+        # hard reset
+        ops.lib._libraries = None
+
+        # sanity check that ops.lib.use works
+        baz = ops.lib.use('baz', 2, 'alice@example.com')
+        self.assertEqual(baz.LIBAPI, 2)
+
     def test_others_found(self):
         tmpdir = self._mkdtemp()
         sys.path = [tmpdir]
@@ -474,6 +492,7 @@ class TestLibFunctional(TestCase):
         LIBAUTHOR = "alice@example.com"
         """))
 
+        # reload
         ops.lib.autoimport()
 
         # sanity check that ops.lib.use works