Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump build from 0.10.0 to 1.0.0 #6450

Closed
wants to merge 11 commits into from
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ texttable==1.6.7
tornado==6.3.3
toml==0.10.2
typing_inspect==0.9.0
build==0.10.0
build==1.0.0
ruamel.yaml==0.17.32

# Optional import in code
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

requires = [
"asyncpg~=0.25",
"build~=1.0",
"click-plugins~=1.0",
# click has been known to publish non-backwards compatible minors in the past (removed deprecated code in 8.1.0)
"click>=8.0,<8.2",
Expand Down Expand Up @@ -34,7 +35,6 @@
"tornado~=6.0",
# lower bound because of ilevkivskyi/typing_inspect#100
"typing_inspect~=0.9",
"build~=0.7",
"ruamel.yaml~=0.17",
"toml~=0.10 ",
]
Expand Down
46 changes: 21 additions & 25 deletions src/inmanta/moduletool.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@
from collections import abc
from configparser import ConfigParser
from functools import total_ordering
from types import TracebackType
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Pattern, Set, Type
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Pattern, Set

import click
import more_itertools
Expand All @@ -46,11 +45,11 @@
from pkg_resources import Requirement, parse_version

import build
import build as build_module # additionaly import as an alias for use in contexts with a name clash
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This I don't understand? why not import it once with the build_module alias and use build_module everywhere?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly because I have a strong aversion to it. Would you prefer it like that? I must confess I have an aversion to the approach I took as well, just less so than the alternative. But I am definitely not too attached to it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why you don't like using the alias everywhere? I think I prefer having the same name everywhere

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Transparency mostly: I prefer to refer to modules by their name. Especially considering there are subimports. Perhaps I should just go back to non-qualified imports. Also less transparent, but at least it's consistent and unambiguous.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

import build.env
import inmanta
import inmanta.warnings
import toml
from build.env import IsolatedEnvBuilder
from inmanta import const, env
from inmanta.ast import CompilerException
from inmanta.command import CLIException, ShowUsageException
Expand Down Expand Up @@ -1524,7 +1523,7 @@ def __str__(self) -> str:
BUILD_FILE_IGNORE_PATTERN: Pattern[str] = re.compile("|".join(("__pycache__", "__cfcache__", r".*\.pyc", rf"{CF_CACHE_DIR}")))


class IsolatedEnvBuilderCached(IsolatedEnvBuilder):
class DefaultIsolatedEnvCached(build.env.DefaultIsolatedEnv):
"""
An IsolatedEnvBuilder that maintains its build environment across invocations of the context manager.
This class is only used by the test suite. It decreases the runtime of the test suite because the build
Expand All @@ -1533,31 +1532,31 @@ class IsolatedEnvBuilderCached(IsolatedEnvBuilder):
This class is a singleton. The get_instance() method should be used to obtain an instance of this class.
"""

_instance: Optional["IsolatedEnvBuilderCached"] = None
_instance: Optional["DefaultIsolatedEnvCached"] = None

def __init__(self) -> None:
super().__init__()
self._isolated_env: Optional[build.env.IsolatedEnv] = None
self._isolated_env: Optional[build.env.DefaultIsolatedEnv] = None

@classmethod
def get_instance(cls) -> "IsolatedEnvBuilderCached":
def get_instance(cls) -> "DefaultIsolatedEnvCached":
"""
This method should be used to obtain an instance of this class, because this class is a singleton.
"""
if not cls._instance:
cls._instance = cls()
return cls._instance

def __enter__(self) -> build.env.IsolatedEnv:
def __enter__(self) -> build.env.DefaultIsolatedEnv:
if not self._isolated_env:
self._isolated_env = super(IsolatedEnvBuilderCached, self).__enter__()
self._isolated_env = super(DefaultIsolatedEnvCached, self).__enter__()
self._install_build_requirements(self._isolated_env)
# All build dependencies are installed, so we can disable the install() method on self._isolated_env.
# This prevents unnecessary pip processes from being spawned.
setattr(self._isolated_env, "install", lambda *args, **kwargs: None)
return self._isolated_env

def _install_build_requirements(self, isolated_env: build.env.IsolatedEnv) -> None:
def _install_build_requirements(self, isolated_env: build.env.DefaultIsolatedEnv) -> None:
"""
Install the build requirements required to build the modules present in the tests/data/modules_v2 directory.
"""
Expand All @@ -1576,16 +1575,13 @@ def _install_build_requirements(self, isolated_env: build.env.IsolatedEnv) -> No
"""
)
builder = build.ProjectBuilder(
srcdir=tmp_python_project_dir,
python_executable=self._isolated_env.executable,
scripts_dir=self._isolated_env.scripts_dir,
source_dir=tmp_python_project_dir,
python_executable=self._isolated_env.python_executable,
)
isolated_env.install(builder.build_system_requires)
isolated_env.install(builder.get_requires_for_build(distribution="wheel"))

def __exit__(
self, exc_type: Optional[Type[BaseException]], exc_val: Optional[BaseException], exc_tb: Optional[TracebackType]
) -> None:
def __exit__(self, *args: object) -> None:
# Ignore implementation from the super class to keep the environment
pass

Expand All @@ -1594,12 +1590,12 @@ def destroy(self) -> None:
Cleanup the cached build environment. It should be called at the end of the test suite.
"""
if self._isolated_env:
super(IsolatedEnvBuilderCached, self).__exit__(None, None, None)
super(DefaultIsolatedEnvCached, self).__exit__()
self._isolated_env = None


class V2ModuleBuilder:
DISABLE_ISOLATED_ENV_BUILDER_CACHE: bool = False
DISABLE_DEFAULT_ISOLATED_ENV_CACHED: bool = False

def __init__(self, module_path: str) -> None:
"""
Expand Down Expand Up @@ -1765,17 +1761,17 @@ def _move_data_files_into_namespace_package_dir(self, build_path: str) -> None:
metadata_file = os.path.join(build_path, "setup.cfg")
shutil.copy(metadata_file, python_pkg_dir)

def _get_isolated_env_builder(self) -> IsolatedEnvBuilder:
def _get_isolated_env_builder(self) -> build_module.env.DefaultIsolatedEnv:
"""
Returns the IsolatedEnvBuilder instance that should be used to build V2 modules. To speed to up the test
Returns the DefaultIsolatedEnv instance that should be used to build V2 modules. To speed to up the test
suite, the build environment is cached when the tests are ran. This is possible because all modules, built
by the test suite, have the same build requirements. For tests that need to test the code path used in
production, the V2ModuleBuilder.DISABLE_ISOLATED_ENV_BUILDER_CACHE flag can be set to True.
production, the V2ModuleBuilder.DISABLE_DEFAULT_ISOLATED_ENV_CACHED flag can be set to True.
"""
if inmanta.RUNNING_TESTS and not V2ModuleBuilder.DISABLE_ISOLATED_ENV_BUILDER_CACHE:
return IsolatedEnvBuilderCached.get_instance()
if inmanta.RUNNING_TESTS and not V2ModuleBuilder.DISABLE_DEFAULT_ISOLATED_ENV_CACHED:
return DefaultIsolatedEnvCached.get_instance()
else:
return IsolatedEnvBuilder()
return build.env.DefaultIsolatedEnv()

def _build_v2_module(self, build_path: str, output_directory: str) -> str:
"""
Expand All @@ -1784,7 +1780,7 @@ def _build_v2_module(self, build_path: str, output_directory: str) -> str:
try:
with self._get_isolated_env_builder() as env:
distribution = "wheel"
builder = build.ProjectBuilder(srcdir=build_path, python_executable=env.executable, scripts_dir=env.scripts_dir)
builder = build.ProjectBuilder(source_dir=build_path, python_executable=env.python_executable)
env.install(builder.build_system_requires)
env.install(builder.get_requires_for_build(distribution=distribution))
return builder.build(distribution=distribution, output_directory=output_directory)
Expand Down
10 changes: 5 additions & 5 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@
from inmanta.env import LocalPackagePath, VirtualEnv, mock_process_env
from inmanta.export import ResourceDict, cfg_env, unknown_parameters
from inmanta.module import InmantaModuleRequirement, InstallMode, Project, RelationPrecedenceRule
from inmanta.moduletool import IsolatedEnvBuilderCached, ModuleTool, V2ModuleBuilder
from inmanta.moduletool import DefaultIsolatedEnvCached, ModuleTool, V2ModuleBuilder
from inmanta.parser.plyInmantaParser import cache_manager
from inmanta.protocol import VersionMatch
from inmanta.server import SLICE_AGENT_MANAGER, SLICE_COMPILER
Expand Down Expand Up @@ -559,7 +559,7 @@ def clean_reset_session():
Execute cleanup tasks that should only run at the end of the test suite.
"""
yield
IsolatedEnvBuilderCached.get_instance().destroy()
DefaultIsolatedEnvCached.get_instance().destroy()


def reset_all_objects():
Expand All @@ -572,15 +572,15 @@ def reset_all_objects():
Project._project = None
unknown_parameters.clear()
InmantaBootloader.AVAILABLE_EXTENSIONS = None
V2ModuleBuilder.DISABLE_ISOLATED_ENV_BUILDER_CACHE = False
V2ModuleBuilder.DISABLE_DEFAULT_ISOLATED_ENV_CACHED = False
compiler.Finalizers.reset_finalizers()
AuthJWTConfig.reset()
InmantaLoggerConfig.clean_instance()


@pytest.fixture()
def disable_isolated_env_builder_cache() -> None:
V2ModuleBuilder.DISABLE_ISOLATED_ENV_BUILDER_CACHE = True
V2ModuleBuilder.DISABLE_DEFAULT_ISOLATED_ENV_CACHED = True


@pytest.fixture(scope="function", autouse=True)
Expand Down Expand Up @@ -1721,7 +1721,7 @@ async def migrate() -> None:
@pytest.fixture(scope="session", autouse=not PYTEST_PLUGIN_MODE)
def guard_invariant_on_v2_modules_in_data_dir(modules_v2_dir: str) -> None:
"""
When the test suite runs, the python environment used to build V2 modules is cached using the IsolatedEnvBuilderCached
When the test suite runs, the python environment used to build V2 modules is cached using the DefaultIsolatedEnvCached
class. This cache relies on the fact that all modules in the tests/data/modules_v2 directory use the same build-backand
and build requirements. This guard verifies whether that assumption is fulfilled and raises an exception if it's not.
"""
Expand Down
6 changes: 2 additions & 4 deletions tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -455,10 +455,8 @@ def create_python_package(
if install:
env.process_env.install_from_source([env.LocalPackagePath(path=path, editable=editable)])
if publish_index is not None:
with build.env.IsolatedEnvBuilder() as build_env:
builder = build.ProjectBuilder(
srcdir=path, python_executable=build_env.executable, scripts_dir=build_env.scripts_dir
)
with build.env.DefaultIsolatedEnv() as build_env:
builder = build.ProjectBuilder(source_dir=path, python_executable=build_env.python_executable)
build_env.install(builder.build_system_requires)
build_env.install(builder.get_requires_for_build(distribution="wheel"))
builder.build(distribution="wheel", output_directory=publish_index.artifact_dir)
Expand Down