From 5c4d8612308fb0956b620cd786a0ed99c9322a16 Mon Sep 17 00:00:00 2001 From: Ethan Paul <24588726+enpaul@users.noreply.github.com> Date: Tue, 13 Aug 2024 13:44:25 -0400 Subject: [PATCH] Consolidate all package handling logic into hook module This creates a large module to sort through, but the hope is that it avoids the need to constantly hop around without rhyme or reason to find the piece of logic you're looking for. The module structure is mapped to functionality rather than an arbitrary concept of reducing line number. --- tests/fixtures.py | 8 +- tests/test_installer.py | 25 ++-- tests/test_transients.py | 26 +++- .../_tox_on_install_helpers.py} | 128 ++++++++++++++---- tox_poetry_installer/hooks/tox_on_install.py | 30 ++-- tox_poetry_installer/installer.py | 88 ------------ 6 files changed, 157 insertions(+), 148 deletions(-) rename tox_poetry_installer/{utilities.py => hooks/_tox_on_install_helpers.py} (78%) delete mode 100644 tox_poetry_installer/installer.py diff --git a/tests/fixtures.py b/tests/fixtures.py index 9bf3b72..f2c9f86 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -10,7 +10,7 @@ import pytest import tox.tox_env.python.virtual_env.runner from poetry.installation.operations.operation import Operation -from tox_poetry_installer import utilities +import tox_poetry_installer.hooks._tox_on_install_helpers TEST_PROJECT_PATH = Path(__file__).parent.resolve() / "test-project" @@ -47,7 +47,11 @@ class MockExecutor: @pytest.fixture def mock_venv(monkeypatch): - monkeypatch.setattr(utilities, "convert_virtualenv", lambda venv: venv) + monkeypatch.setattr( + tox_poetry_installer.hooks._tox_on_install_helpers, + "convert_virtualenv", + lambda venv: venv, + ) monkeypatch.setattr(poetry.installation.executor, "Executor", MockExecutor) monkeypatch.setattr( tox.tox_env.python.virtual_env.runner, "VirtualEnvRunner", MockVirtualEnv diff --git a/tests/test_installer.py b/tests/test_installer.py index 517879c..5a4cd28 100644 --- a/tests/test_installer.py +++ b/tests/test_installer.py @@ -6,23 +6,24 @@ import pytest import tox.tox_env.python.virtual_env.runner from poetry.factory import Factory +import tox_poetry_installer.hooks._tox_on_install_helpers from .fixtures import mock_poetry_factory from .fixtures import mock_venv -from tox_poetry_installer import installer -from tox_poetry_installer import utilities def test_deduplication(mock_venv, mock_poetry_factory): """Test that the installer does not install duplicate dependencies""" poetry = Factory().create_poetry(None) - packages: utilities.PackageMap = { + packages: tox_poetry_installer.hooks._tox_on_install_helpers.PackageMap = { item.name: item for item in poetry.locker.locked_repository().packages } venv = tox.tox_env.python.virtual_env.runner.VirtualEnvRunner() to_install = [packages["toml"], packages["toml"]] - installer.install(poetry, venv, to_install) + tox_poetry_installer.hooks._tox_on_install_helpers.install_package( + poetry, venv, to_install + ) assert len(set(to_install)) == len(venv.installed) # pylint: disable=no-member @@ -30,7 +31,7 @@ def test_deduplication(mock_venv, mock_poetry_factory): def test_parallelization(mock_venv, mock_poetry_factory): """Test that behavior is consistent between parallel and non-parallel usage""" poetry = Factory().create_poetry(None) - packages: utilities.PackageMap = { + packages: tox_poetry_installer.hooks._tox_on_install_helpers.PackageMap = { item.name: item for item in poetry.locker.locked_repository().packages } @@ -45,12 +46,16 @@ def test_parallelization(mock_venv, mock_poetry_factory): venv_sequential = tox.tox_env.python.virtual_env.runner.VirtualEnvRunner() start_sequential = time.time() - installer.install(poetry, venv_sequential, to_install, 0) + tox_poetry_installer.hooks._tox_on_install_helpers.install_package( + poetry, venv_sequential, to_install, 0 + ) sequential = time.time() - start_sequential venv_parallel = tox.tox_env.python.virtual_env.runner.VirtualEnvRunner() start_parallel = time.time() - installer.install(poetry, venv_parallel, to_install, 5) + tox_poetry_installer.hooks._tox_on_install_helpers.install_package( + poetry, venv_parallel, to_install, 5 + ) parallel = time.time() - start_parallel # The mock delay during package install is static (one second) so these values should all @@ -72,7 +77,7 @@ def test_propagates_exceptions_during_installation( from tox_poetry_installer import _poetry # pylint: disable=import-outside-toplevel poetry = Factory().create_poetry(None) - packages: utilities.PackageMap = { + packages: tox_poetry_installer.hooks._tox_on_install_helpers.PackageMap = { item.name: item for item in poetry.locker.locked_repository().packages } to_install = [packages["toml"]] @@ -85,6 +90,8 @@ def test_propagates_exceptions_during_installation( **{"return_value.execute.side_effect": fake_exception}, ): with pytest.raises(ValueError) as exc_info: - installer.install(poetry, venv, to_install, num_threads) + tox_poetry_installer.hooks._tox_on_install_helpers.install_package( + poetry, venv, to_install, num_threads + ) assert exc_info.value is fake_exception diff --git a/tests/test_transients.py b/tests/test_transients.py index 2fa573f..cd883ff 100644 --- a/tests/test_transients.py +++ b/tests/test_transients.py @@ -4,19 +4,21 @@ import poetry.utils.env import pytest from poetry.puzzle.provider import Provider +import tox_poetry_installer.hooks._tox_on_install_helpers from .fixtures import mock_poetry_factory from .fixtures import mock_venv from tox_poetry_installer import constants from tox_poetry_installer import exceptions -from tox_poetry_installer import utilities def test_allow_missing(): """Test that the ``allow_missing`` parameter works as expected""" with pytest.raises(exceptions.LockedDepNotFoundError): - utilities.identify_transients("luke-skywalker", {}, None) + tox_poetry_installer.hooks._tox_on_install_helpers.identify_transients( + "luke-skywalker", {}, None + ) - assert not utilities.identify_transients( + assert not tox_poetry_installer.hooks._tox_on_install_helpers.identify_transients( "darth-vader", {}, None, allow_missing=["darth-vader"] ) @@ -36,7 +38,9 @@ def test_exclude_pep508(): "=>foo", ]: with pytest.raises(exceptions.LockedDepVersionConflictError): - utilities.identify_transients(version, {}, None) + tox_poetry_installer.hooks._tox_on_install_helpers.identify_transients( + version, {}, None + ) def test_functional(mock_poetry_factory, mock_venv): @@ -46,7 +50,9 @@ def test_functional(mock_poetry_factory, mock_venv): is always the last in the returned list. """ pypoetry = poetry.factory.Factory().create_poetry(None) - packages = utilities.build_package_map(pypoetry) + packages = tox_poetry_installer.hooks._tox_on_install_helpers.build_package_map( + pypoetry + ) venv = poetry.utils.env.VirtualEnv() # pylint: disable=no-value-for-parameter requests_requires = [ @@ -57,12 +63,18 @@ def test_functional(mock_poetry_factory, mock_venv): packages["requests"][0], ] - transients = utilities.identify_transients("requests", packages, venv) + transients = tox_poetry_installer.hooks._tox_on_install_helpers.identify_transients( + "requests", packages, venv + ) assert all((item in requests_requires) for item in transients) assert all((item in transients) for item in requests_requires) for package in [packages["requests"][0], packages["tox"][0], packages["flask"][0]]: - transients = utilities.identify_transients(package.name, packages, venv) + transients = ( + tox_poetry_installer.hooks._tox_on_install_helpers.identify_transients( + package.name, packages, venv + ) + ) assert transients[-1] == package assert len(transients) == len(set(transients)) diff --git a/tox_poetry_installer/utilities.py b/tox_poetry_installer/hooks/_tox_on_install_helpers.py similarity index 78% rename from tox_poetry_installer/utilities.py rename to tox_poetry_installer/hooks/_tox_on_install_helpers.py index 2bfeda8..b02a515 100644 --- a/tox_poetry_installer/utilities.py +++ b/tox_poetry_installer/hooks/_tox_on_install_helpers.py @@ -1,10 +1,11 @@ -"""Helper utility functions, usually bridging Tox and Poetry functionality""" -# Silence this one globally to support the internal function imports for the proxied poetry module. -# See the docstring in 'tox_poetry_installer._poetry' for more context. -# pylint: disable=import-outside-toplevel +"""Helper functions for the :func:`tox_on_install` hook""" import collections +import concurrent.futures +import contextlib import typing +from datetime import datetime from pathlib import Path +from typing import Collection from typing import Dict from typing import List from typing import Sequence @@ -22,7 +23,6 @@ from tox_poetry_installer import logger if typing.TYPE_CHECKING: from tox_poetry_installer import _poetry - PackageMap = Dict[str, List[PoetryPackage]] @@ -52,30 +52,6 @@ def check_preconditions(venv: ToxVirtualEnv) -> "_poetry.Poetry": ) from None -def convert_virtualenv(venv: ToxVirtualEnv) -> "_poetry.VirtualEnv": - """Convert a Tox venv to a Poetry venv - - :param venv: Tox ``VirtualEnv`` object representing a tox virtual environment - :returns: Poetry ``VirtualEnv`` object representing a poetry virtual environment - """ - from tox_poetry_installer import _poetry - - return _poetry.VirtualEnv(path=Path(venv.env_dir)) - - -def build_package_map(poetry: "_poetry.Poetry") -> PackageMap: - """Build the mapping of package names to objects - - :param poetry: Populated poetry object to load locked packages from - :returns: Mapping of package names to Poetry package objects - """ - packages = collections.defaultdict(list) - for package in poetry.locker.locked_repository().packages: - packages[package.name].append(package) - - return packages - - def identify_transients( dep_name: str, packages: PackageMap, @@ -264,6 +240,76 @@ def find_dev_deps( return dedupe_packages(dev_group_deps + legacy_dev_group_deps) +def install_package( + poetry: "_poetry.Poetry", + venv: ToxVirtualEnv, + packages: Collection["_poetry.PoetryPackage"], + parallels: int = 0, +): + """Install a bunch of packages to a virtualenv + + :param poetry: Poetry object the packages were sourced from + :param venv: Tox virtual environment to install the packages to + :param packages: List of packages to install to the virtual environment + :param parallels: Number of parallel processes to use for installing dependency packages, or + ``None`` to disable parallelization. + """ + from tox_poetry_installer import _poetry + + logger.info(f"Installing {len(packages)} packages to environment at {venv.env_dir}") + + install_executor = _poetry.Executor( + env=convert_virtualenv(venv), + io=_poetry.NullIO(), + pool=poetry.pool, + config=_poetry.Config(), + ) + + installed: Set[_poetry.PoetryPackage] = set() + + def logged_install(dependency: _poetry.PoetryPackage) -> None: + start = datetime.now() + logger.debug(f"Installing {dependency}") + install_executor.execute([_poetry.Install(package=dependency)]) + end = datetime.now() + logger.debug(f"Finished installing {dependency} in {end - start}") + + @contextlib.contextmanager + def _optional_parallelize(): + """A bit of cheat, really + + A context manager that exposes a common interface for the caller that optionally + enables/disables the usage of the parallel thread pooler depending on the value of + the ``parallels`` parameter. + """ + if parallels > 0: + with concurrent.futures.ThreadPoolExecutor( + max_workers=parallels + ) as executor: + yield executor.submit + else: + yield lambda func, arg: func(arg) + + with _optional_parallelize() as executor: + futures = [] + for dependency in packages: + if dependency not in installed: + installed.add(dependency) + logger.debug(f"Queuing {dependency}") + future = executor(logged_install, dependency) + if future is not None: + futures.append(future) + else: + logger.debug(f"Skipping {dependency}, already installed") + logger.debug("Waiting for installs to finish...") + + for future in concurrent.futures.as_completed(futures): + # Don't actually care about the return value, just waiting on the + # future to ensure any exceptions that were raised in the called + # function are propagated. + future.result() + + def dedupe_packages(packages: Sequence[PoetryPackage]) -> List[PoetryPackage]: """Deduplicates a sequence of PoetryPackages while preserving ordering @@ -273,3 +319,27 @@ def dedupe_packages(packages: Sequence[PoetryPackage]) -> List[PoetryPackage]: # Make this faster, avoid method lookup below seen_add = seen.add return [p for p in packages if not (p in seen or seen_add(p))] + + +def convert_virtualenv(venv: ToxVirtualEnv) -> "_poetry.VirtualEnv": + """Convert a Tox venv to a Poetry venv + + :param venv: Tox ``VirtualEnv`` object representing a tox virtual environment + :returns: Poetry ``VirtualEnv`` object representing a poetry virtual environment + """ + from tox_poetry_installer import _poetry + + return _poetry.VirtualEnv(path=Path(venv.env_dir)) + + +def build_package_map(poetry: "_poetry.Poetry") -> PackageMap: + """Build the mapping of package names to objects + + :param poetry: Populated poetry object to load locked packages from + :returns: Mapping of package names to Poetry package objects + """ + packages = collections.defaultdict(list) + for package in poetry.locker.locked_repository().packages: + packages[package.name].append(package) + + return packages diff --git a/tox_poetry_installer/hooks/tox_on_install.py b/tox_poetry_installer/hooks/tox_on_install.py index cf13a94..236dd70 100644 --- a/tox_poetry_installer/hooks/tox_on_install.py +++ b/tox_poetry_installer/hooks/tox_on_install.py @@ -10,9 +10,15 @@ from tox.plugin import impl from tox.tox_env.api import ToxEnv as ToxVirtualEnv from tox_poetry_installer import exceptions -from tox_poetry_installer import installer from tox_poetry_installer import logger -from tox_poetry_installer import utilities +from tox_poetry_installer.hooks._tox_on_install_helpers import build_package_map +from tox_poetry_installer.hooks._tox_on_install_helpers import check_preconditions +from tox_poetry_installer.hooks._tox_on_install_helpers import convert_virtualenv +from tox_poetry_installer.hooks._tox_on_install_helpers import dedupe_packages +from tox_poetry_installer.hooks._tox_on_install_helpers import find_additional_deps +from tox_poetry_installer.hooks._tox_on_install_helpers import find_group_deps +from tox_poetry_installer.hooks._tox_on_install_helpers import find_project_deps +from tox_poetry_installer.hooks._tox_on_install_helpers import install_package @impl @@ -20,7 +26,7 @@ def tox_on_install( tox_env: ToxVirtualEnv, section: str # pylint: disable=unused-argument ) -> None: try: - poetry = utilities.check_preconditions(tox_env) + poetry = check_preconditions(tox_env) except exceptions.SkipEnvironment as err: if ( isinstance(err, exceptions.PoetryNotInstalledError) @@ -33,7 +39,7 @@ def tox_on_install( logger.info(f"Loaded project pyproject.toml from {poetry.file}") - virtualenv = utilities.convert_virtualenv(tox_env) + virtualenv = convert_virtualenv(tox_env) if not poetry.locker.is_fresh(): logger.warning( @@ -46,13 +52,13 @@ def tox_on_install( f"Unlocked dependencies '{tox_env.conf['deps']}' specified for environment '{tox_env.name}' which requires locked dependencies" ) - packages = utilities.build_package_map(poetry) + packages = build_package_map(poetry) - group_deps = utilities.dedupe_packages( + group_deps = dedupe_packages( list( chain( *[ - utilities.find_group_deps(group, packages, virtualenv, poetry) + find_group_deps(group, packages, virtualenv, poetry) for group in tox_env.conf["poetry_dep_groups"] ] ) @@ -62,7 +68,7 @@ def tox_on_install( f"Identified {len(group_deps)} group dependencies to install to env" ) - env_deps = utilities.find_additional_deps( + env_deps = find_additional_deps( packages, virtualenv, poetry, tox_env.conf["locked_deps"] ) @@ -77,9 +83,7 @@ def tox_on_install( extras = [] if tox_env.conf["install_project_deps"]: - project_deps = utilities.find_project_deps( - packages, virtualenv, poetry, extras - ) + project_deps = find_project_deps(packages, virtualenv, poetry, extras) logger.info( f"Identified {len(project_deps)} project dependencies to install to env" ) @@ -93,10 +97,10 @@ def tox_on_install( logger.error(f"Internal plugin error: {err}") raise err - dependencies = utilities.dedupe_packages(group_deps + env_deps + project_deps) + dependencies = dedupe_packages(group_deps + env_deps + project_deps) logger.info(f"Installing {len(dependencies)} dependencies from Poetry lock file") - installer.install( + install_package( poetry, tox_env, dependencies, diff --git a/tox_poetry_installer/installer.py b/tox_poetry_installer/installer.py deleted file mode 100644 index 60d3a7b..0000000 --- a/tox_poetry_installer/installer.py +++ /dev/null @@ -1,88 +0,0 @@ -"""Funcationality for performing virtualenv installation""" -# Silence this one globally to support the internal function imports for the proxied poetry module. -# See the docstring in 'tox_poetry_installer._poetry' for more context. -# pylint: disable=import-outside-toplevel -import concurrent.futures -import contextlib -import typing -from datetime import datetime -from typing import Collection -from typing import Set - -from tox.tox_env.api import ToxEnv as ToxVirtualEnv - -from tox_poetry_installer import logger -from tox_poetry_installer import utilities - -if typing.TYPE_CHECKING: - from tox_poetry_installer import _poetry - - -def install( - poetry: "_poetry.Poetry", - venv: ToxVirtualEnv, - packages: Collection["_poetry.PoetryPackage"], - parallels: int = 0, -): - """Install a bunch of packages to a virtualenv - - :param poetry: Poetry object the packages were sourced from - :param venv: Tox virtual environment to install the packages to - :param packages: List of packages to install to the virtual environment - :param parallels: Number of parallel processes to use for installing dependency packages, or - ``None`` to disable parallelization. - """ - from tox_poetry_installer import _poetry - - logger.info(f"Installing {len(packages)} packages to environment at {venv.env_dir}") - - install_executor = _poetry.Executor( - env=utilities.convert_virtualenv(venv), - io=_poetry.NullIO(), - pool=poetry.pool, - config=_poetry.Config(), - ) - - installed: Set[_poetry.PoetryPackage] = set() - - def logged_install(dependency: _poetry.PoetryPackage) -> None: - start = datetime.now() - logger.debug(f"Installing {dependency}") - install_executor.execute([_poetry.Install(package=dependency)]) - end = datetime.now() - logger.debug(f"Finished installing {dependency} in {end - start}") - - @contextlib.contextmanager - def _optional_parallelize(): - """A bit of cheat, really - - A context manager that exposes a common interface for the caller that optionally - enables/disables the usage of the parallel thread pooler depending on the value of - the ``parallels`` parameter. - """ - if parallels > 0: - with concurrent.futures.ThreadPoolExecutor( - max_workers=parallels - ) as executor: - yield executor.submit - else: - yield lambda func, arg: func(arg) - - with _optional_parallelize() as executor: - futures = [] - for dependency in packages: - if dependency not in installed: - installed.add(dependency) - logger.debug(f"Queuing {dependency}") - future = executor(logged_install, dependency) - if future is not None: - futures.append(future) - else: - logger.debug(f"Skipping {dependency}, already installed") - logger.debug("Waiting for installs to finish...") - - for future in concurrent.futures.as_completed(futures): - # Don't actually care about the return value, just waiting on the - # future to ensure any exceptions that were raised in the called - # function are propagated. - future.result()