Has setup_logging not override already setup loging level. (#132)

* Has setup_logging not override already setup loging level.

Add a verbosity=None (and make it the default) for verbosity.
If one has already increase the logging level, the default value of
verbose should not change it. This is achieved with a default value of
None.

Technically I guess we should have a context manager to restaure
previous value if True/False is passed, but that will be for another
time.

I also think that if verbosity is an int, we should set the lgging level
to that int instead of havin an arbitrary verbosity of 0/1/2; but that's
also another discussion.

I'm doing this as I'd like adding some logger.debug, but I don't want to
pass verbose=True everywhere.

Restore logger level between calls

forward getattr/setattr

unconditional context manager

* Apply suggestions from code review

micropip/_commands/install.py

@@ -11,7 +11,7 @@ async def install(
     pre: bool = False,
     index_urls: list[str] | str | None = None,
     *,
-    verbose: bool | int = False,
+    verbose: bool | int | None = None,
 ) -> None:
     if index_urls is None:
         index_urls = package_index.INDEX_URLS[:]

micropip/_commands/uninstall.py

@@ -28,76 +28,78 @@ def uninstall(packages: str | list[str], *, verbose: bool | int = False) -> None
         By default, micropip is silent. Setting ``verbose=True`` will print
         similar information as pip.
     """
-    logger = setup_logging(verbose)
-
-    if isinstance(packages, str):
-        packages = [packages]
-
-    distributions: list[Distribution] = []
-    for package in packages:
-        try:
-            dist = importlib.metadata.distribution(package)
-            distributions.append(dist)
-        except importlib.metadata.PackageNotFoundError:
-            logger.warning("Skipping '%s' as it is not installed.", package)
-
-    for dist in distributions:
-        # Note: this value needs to be retrieved before removing files, as
-        #       dist.name uses metadata file to get the name
-        name = dist.name
-        version = dist.version
-
-        logger.info("Found existing installation: %s %s", name, version)
-
-        root = get_root(dist)
-        files = get_files_in_distribution(dist)
-        directories = set()
-
-        for file in files:
-            if not file.is_file():
-                if not file.is_relative_to(root):
-                    # This file is not in the site-packages directory. Probably one of:
-                    # - data_files
-                    # - scripts
-                    # - entry_points
-                    # Since we don't support these, we can ignore them (except for data_files (TODO))
+    with setup_logging().ctx_level(verbose) as logger:
+
+        if isinstance(packages, str):
+            packages = [packages]
+
+        distributions: list[Distribution] = []
+        for package in packages:
+            try:
+                dist = importlib.metadata.distribution(package)
+                distributions.append(dist)
+            except importlib.metadata.PackageNotFoundError:
+                logger.warning("Skipping '%s' as it is not installed.", package)
+
+        for dist in distributions:
+            # Note: this value needs to be retrieved before removing files, as
+            #       dist.name uses metadata file to get the name
+            name = dist.name
+            version = dist.version
+
+            logger.info("Found existing installation: %s %s", name, version)
+
+            root = get_root(dist)
+            files = get_files_in_distribution(dist)
+            directories = set()
+
+            for file in files:
+                if not file.is_file():
+                    if not file.is_relative_to(root):
+                        # This file is not in the site-packages directory. Probably one of:
+                        # - data_files
+                        # - scripts
+                        # - entry_points
+                        # Since we don't support these, we can ignore them (except for data_files (TODO))
+                        logger.warning(
+                            "skipping file '%s' that is relative to root",
+                        )
+                        continue
+                    # see PR 130, it is likely that this is never triggered since Python 3.12
+                    # as non existing files are not listed by get_files_in_distribution anymore.
                     logger.warning(
-                        "skipping file '%s' that is relative to root",
+                        "A file '%s' listed in the metadata of '%s' does not exist.",
+                        file,
+                        name,
                     )
+
                     continue
-                # see PR 130, it is likely that this is never triggered since Python 3.12
-                # as non existing files are not listed by get_files_in_distribution anymore.
-                logger.warning(
-                    "A file '%s' listed in the metadata of '%s' does not exist.",
-                    file,
-                    name,
-                )
 
-                continue
+                file.unlink()
 
-            file.unlink()
+                if file.parent != root:
+                    directories.add(file.parent)
 
-            if file.parent != root:
-                directories.add(file.parent)
+            # Remove directories in reverse hierarchical order
+            for directory in sorted(
+                directories, key=lambda x: len(x.parts), reverse=True
+            ):
+                try:
+                    directory.rmdir()
+                except OSError:
+                    logger.warning(
+                        "A directory '%s' is not empty after uninstallation of '%s'. "
+                        "This might cause problems when installing a new version of the package. ",
+                        directory,
+                        name,
+                    )
 
-        # Remove directories in reverse hierarchical order
-        for directory in sorted(directories, key=lambda x: len(x.parts), reverse=True):
-            try:
-                directory.rmdir()
-            except OSError:
-                logger.warning(
-                    "A directory '%s' is not empty after uninstallation of '%s'. "
-                    "This might cause problems when installing a new version of the package. ",
-                    directory,
-                    name,
-                )
-
-        if hasattr(loadedPackages, name):
-            delattr(loadedPackages, name)
-        else:
-            # This should not happen, but just in case
-            logger.warning("a package '%s' was not found in loadedPackages.", name)
-
-        logger.info("Successfully uninstalled %s-%s", name, version)
-
-    importlib.invalidate_caches()
+            if hasattr(loadedPackages, name):
+                delattr(loadedPackages, name)
+            else:
+                # This should not happen, but just in case
+                logger.warning("a package '%s' was not found in loadedPackages.", name)
+
+            logger.info("Successfully uninstalled %s-%s", name, version)
+
+        importlib.invalidate_caches()

micropip/install.py

@@ -21,7 +21,7 @@ async def install(
     pre: bool = False,
     index_urls: list[str] | str | None = None,
     *,
-    verbose: bool | int = False,
+    verbose: bool | int | None = None,
 ) -> None:
     """Install the given package and all of its dependencies.
 
@@ -106,82 +106,82 @@ async def install(
           it finds a package. If no package is found, an error will be raised.
 
     verbose :
-        Print more information about the process.
-        By default, micropip is silent. Setting ``verbose=True`` will print
-        similar information as pip.
+        Print more information about the process. By default, micropip does not
+        change logger level. Setting ``verbose=True`` will print similar
+        information as pip.
     """
-    logger = setup_logging(verbose)
-
-    ctx = default_environment()
-    if isinstance(requirements, str):
-        requirements = [requirements]
-
-    fetch_kwargs = dict()
-
-    if credentials:
-        fetch_kwargs["credentials"] = credentials
-
-    # Note: getsitepackages is not available in a virtual environment...
-    # See https://github.com/pypa/virtualenv/issues/228 (issue is closed but
-    # problem is not fixed)
-    from site import getsitepackages
-
-    wheel_base = Path(getsitepackages()[0])
-
-    if index_urls is None:
-        index_urls = package_index.INDEX_URLS[:]
-
-    transaction = Transaction(
-        ctx=ctx,  # type: ignore[arg-type]
-        ctx_extras=[],
-        keep_going=keep_going,
-        deps=deps,
-        pre=pre,
-        fetch_kwargs=fetch_kwargs,
-        verbose=verbose,
-        index_urls=index_urls,
-    )
-    await transaction.gather_requirements(requirements)
-
-    if transaction.failed:
-        failed_requirements = ", ".join([f"'{req}'" for req in transaction.failed])
-        raise ValueError(
-            f"Can't find a pure Python 3 wheel for: {failed_requirements}\n"
-            f"See: {FAQ_URLS['cant_find_wheel']}\n"
+    with setup_logging().ctx_level(verbose) as logger:
+
+        ctx = default_environment()
+        if isinstance(requirements, str):
+            requirements = [requirements]
+
+        fetch_kwargs = dict()
+
+        if credentials:
+            fetch_kwargs["credentials"] = credentials
+
+        # Note: getsitepackages is not available in a virtual environment...
+        # See https://github.com/pypa/virtualenv/issues/228 (issue is closed but
+        # problem is not fixed)
+        from site import getsitepackages
+
+        wheel_base = Path(getsitepackages()[0])
+
+        if index_urls is None:
+            index_urls = package_index.INDEX_URLS[:]
+
+        transaction = Transaction(
+            ctx=ctx,  # type: ignore[arg-type]
+            ctx_extras=[],
+            keep_going=keep_going,
+            deps=deps,
+            pre=pre,
+            fetch_kwargs=fetch_kwargs,
+            verbose=verbose,
+            index_urls=index_urls,
         )
+        await transaction.gather_requirements(requirements)
 
-    package_names = [pkg.name for pkg in transaction.pyodide_packages] + [
-        pkg.name for pkg in transaction.wheels
-    ]
-
-    if package_names:
-        logger.info("Installing collected packages: %s", ", ".join(package_names))
-
-    wheel_promises: list[Coroutine[Any, Any, None] | asyncio.Task[Any]] = []
-    # Install built-in packages
-    pyodide_packages = transaction.pyodide_packages
-    if len(pyodide_packages):
-        # Note: branch never happens in out-of-browser testing because in
-        # that case REPODATA_PACKAGES is empty.
-        wheel_promises.append(
-            asyncio.ensure_future(
-                loadPackage(to_js([name for [name, _, _] in pyodide_packages]))
+        if transaction.failed:
+            failed_requirements = ", ".join([f"'{req}'" for req in transaction.failed])
+            raise ValueError(
+                f"Can't find a pure Python 3 wheel for: {failed_requirements}\n"
+                f"See: {FAQ_URLS['cant_find_wheel']}\n"
+            )
+
+        package_names = [pkg.name for pkg in transaction.pyodide_packages] + [
+            pkg.name for pkg in transaction.wheels
+        ]
+
+        if package_names:
+            logger.info("Installing collected packages: %s", ", ".join(package_names))
+
+        wheel_promises: list[Coroutine[Any, Any, None] | asyncio.Task[Any]] = []
+        # Install built-in packages
+        pyodide_packages = transaction.pyodide_packages
+        if len(pyodide_packages):
+            # Note: branch never happens in out-of-browser testing because in
+            # that case REPODATA_PACKAGES is empty.
+            wheel_promises.append(
+                asyncio.ensure_future(
+                    loadPackage(to_js([name for [name, _, _] in pyodide_packages]))
+                )
             )
-        )
 
-    # Now install PyPI packages
-    for wheel in transaction.wheels:
-        # detect whether the wheel metadata is from PyPI or from custom location
-        # wheel metadata from PyPI has SHA256 checksum digest.
-        wheel_promises.append(wheel.install(wheel_base))
+        # Now install PyPI packages
+        for wheel in transaction.wheels:
+            # detect whether the wheel metadata is from PyPI or from custom location
+            # wheel metadata from PyPI has SHA256 checksum digest.
+            wheel_promises.append(wheel.install(wheel_base))
 
-    await asyncio.gather(*wheel_promises)
+        await asyncio.gather(*wheel_promises)
 
-    packages = [f"{pkg.name}-{pkg.version}" for pkg in transaction.pyodide_packages] + [
-        f"{pkg.name}-{pkg.version}" for pkg in transaction.wheels
-    ]
+        packages = [
+            f"{pkg.name}-{pkg.version}" for pkg in transaction.pyodide_packages
+        ] + [f"{pkg.name}-{pkg.version}" for pkg in transaction.wheels]
 
-    if packages:
-        logger.info("Successfully installed %s", ", ".join(packages))
+        if packages:
+            logger.info("Successfully installed %s", ", ".join(packages))
 
-    importlib.invalidate_caches()
+        importlib.invalidate_caches()

micropip/logging.py

@@ -1,14 +1,14 @@
-import contextlib
 import logging
 import sys
 from collections.abc import Generator
+from contextlib import contextmanager
 from typing import Any
 
 _logger: logging.Logger | None = None
 _indentation: int = 0
 
 
-@contextlib.contextmanager
+@contextmanager
 def indent_log(num: int = 2) -> Generator[None, None, None]:
     """
     A context manager which will cause the log output to be indented for any
@@ -87,17 +87,56 @@ def _set_formatter_once() -> None:
     _logger.addHandler(ch)
 
 
-def setup_logging(verbosity: int | bool) -> logging.Logger:
+class LoggerWrapper:
+    __slots__ = ("logger", "_orig_level")
+
+    logger: logging.Logger
+
+    def __init__(self, logger: logging.Logger):
+        self.logger = logger
+
+    def __getattr__(self, attr):
+        return getattr(self.logger, attr)
+
+    def __setattr__(self, attr, value):
+        return setattr(self.logger, attr, value)
+
+    @contextmanager
+    def ctx_level(self, verbosity: int | bool | None = None):
+        cur_level = self.logger.level
+        if verbosity is not None:
+            if verbosity > 2:
+                raise ValueError(
+                    "verbosity should be in 0,1,2, False, True, if you are "
+                    "directly setting level using logging.LEVEL, please "
+                    "directly call `setLevel` on the logger."
+                )
+            elif verbosity >= 2:
+                level_number = logging.DEBUG
+            elif verbosity == 1:  # True == 1
+                level_number = logging.INFO
+            else:
+                level_number = logging.WARNING
+            self.logger.setLevel(level_number)
+        try:
+            yield self.logger
+        finally:
+            self.logger.setLevel(cur_level)
+
+
+def setup_logging() -> LoggerWrapper:
     _set_formatter_once()
+    assert _logger
+    return LoggerWrapper(_logger)
+
 
+# TODO: expose this somehow
+def set_log_level(verbosity: int | bool):
     if verbosity >= 2:
         level_number = logging.DEBUG
     elif verbosity == 1:  # True == 1
         level_number = logging.INFO
     else:
         level_number = logging.WARNING
-
     assert _logger
     _logger.setLevel(level_number)
-
-    return _logger