From 9d1cd45530483820610d797898abee313383fbd6 Mon Sep 17 00:00:00 2001 From: Mirko Dietrich Date: Tue, 9 Apr 2024 15:34:46 +0200 Subject: [PATCH] feat: update `PackageBuilder` and add `PackageSource` classes Extract the packager builder logic into separate class `PackageBuilder`. The class `PackageSource` represents a source package folder and handles validation. Build packages to temporary package file, so no output files are overwritten when build fails. Introduce a `--no-interaction` CLI flag, similar to how it's implemented in Poetry and other tools. Also add a `--force` flag for the `package` task. Ref: #69 --- questionpy_sdk/__main__.py | 19 +++- questionpy_sdk/commands/_helper.py | 42 ++----- questionpy_sdk/commands/create.py | 13 ++- questionpy_sdk/commands/package.py | 103 ++++++++--------- questionpy_sdk/commands/repo/structure.py | 2 +- questionpy_sdk/package.py | 27 ----- questionpy_sdk/package/__init__.py | 3 + questionpy_sdk/package/_helper.py | 17 +++ questionpy_sdk/package/builder.py | 130 ++++++++++++++++++++++ questionpy_sdk/package/errors.py | 11 ++ questionpy_sdk/package/source.py | 78 +++++++++++++ 11 files changed, 328 insertions(+), 117 deletions(-) delete mode 100644 questionpy_sdk/package.py create mode 100644 questionpy_sdk/package/__init__.py create mode 100644 questionpy_sdk/package/_helper.py create mode 100644 questionpy_sdk/package/builder.py create mode 100644 questionpy_sdk/package/errors.py create mode 100644 questionpy_sdk/package/source.py diff --git a/questionpy_sdk/__main__.py b/questionpy_sdk/__main__.py index 2ae2a156..bfab60e1 100644 --- a/questionpy_sdk/__main__.py +++ b/questionpy_sdk/__main__.py @@ -3,6 +3,7 @@ # (c) Technische Universität Berlin, innoCampus import logging +import shutil import sys import click @@ -12,11 +13,25 @@ from questionpy_sdk.commands.repo import repo from questionpy_sdk.commands.run import run +# Contrary to click's docs, there's no autodetection of the terminal width (pallets/click#2253) +terminal_width = shutil.get_terminal_size()[0] -@click.group(context_settings={"help_option_names": ["-h", "--help"]}) + +@click.group(context_settings={"help_option_names": ["-h", "--help"], "terminal_width": terminal_width}) +@click.option( + "-n", + "--no-interaction", + is_flag=True, + show_default=True, + default=False, + help="Do not ask any interactive question.", +) @click.option("-v", "--verbose", is_flag=True, show_default=True, default=False, help="Use log level DEBUG.") -def cli(*, verbose: bool) -> None: +@click.pass_context +def cli(ctx: click.Context, *, verbose: bool, no_interaction: bool) -> None: logging.basicConfig(level=logging.DEBUG if verbose else logging.INFO, stream=sys.stderr) + ctx.ensure_object(dict) + ctx.obj["no_interaction"] = no_interaction cli.add_command(create) diff --git a/questionpy_sdk/commands/_helper.py b/questionpy_sdk/commands/_helper.py index 4a5510d4..0da1f7d5 100644 --- a/questionpy_sdk/commands/_helper.py +++ b/questionpy_sdk/commands/_helper.py @@ -6,12 +6,9 @@ from pathlib import Path import click -import yaml -from pydantic import ValidationError -from questionpy_common.manifest import Manifest -from questionpy_sdk.constants import PACKAGE_CONFIG_FILENAME -from questionpy_sdk.models import PackageConfig +from questionpy_sdk.package.errors import PackageSourceValidationError +from questionpy_sdk.package.source import PackageSource from questionpy_server.worker.runtime.package_location import ( DirPackageLocation, FunctionPackageLocation, @@ -20,23 +17,18 @@ ) -def create_normalized_filename(manifest: Manifest) -> str: - """Creates a normalized file name for the given manifest. - - Args: - manifest: manifest of the package - - Returns: - normalized file name - """ - return f"{manifest.namespace}-{manifest.short_name}-{manifest.version}.qpy" +def read_package_source(source_path: Path) -> PackageSource: + try: + return PackageSource(source_path) + except PackageSourceValidationError as exc: + raise click.ClickException(str(exc)) from exc def infer_package_kind(string: str) -> PackageLocation: path = Path(string) if path.is_dir(): - config = read_yaml_config(path / PACKAGE_CONFIG_FILENAME) - return DirPackageLocation(path, config) + package_source = read_package_source(path) + return DirPackageLocation(path, package_source.config) if zipfile.is_zipfile(path): return ZipPackageLocation(path) @@ -62,17 +54,5 @@ def infer_package_kind(string: str) -> PackageLocation: raise click.ClickException(msg) -def read_yaml_config(path: Path) -> PackageConfig: - if not path.is_file(): - msg = f"The config '{path}' does not exist." - raise click.ClickException(msg) - - with path.open() as config_f: - try: - return PackageConfig.model_validate(yaml.safe_load(config_f)) - except yaml.YAMLError as e: - msg = f"Failed to parse config '{path}': {e}" - raise click.ClickException(msg) from e - except ValidationError as e: - msg = f"Invalid config '{path}': {e}" - raise click.ClickException(msg) from e +def confirm_overwrite(filepath: Path) -> bool: + return click.confirm(f"The path '{filepath}' already exists. Do you want to overwrite it?", abort=True) diff --git a/questionpy_sdk/commands/create.py b/questionpy_sdk/commands/create.py index c9dc3b1f..38411f19 100644 --- a/questionpy_sdk/commands/create.py +++ b/questionpy_sdk/commands/create.py @@ -25,9 +25,18 @@ def validate_name(context: click.Context, _parameter: click.Parameter, value: st @click.command(context_settings={"show_default": True}) @click.argument("short_name", callback=validate_name) -@click.option("--namespace", "-n", "namespace", callback=validate_name, default=DEFAULT_NAMESPACE) -@click.option("--out", "-o", "out_path", type=click.Path(path_type=Path)) +@click.option( + "--namespace", "-n", "namespace", callback=validate_name, default=DEFAULT_NAMESPACE, help="Package namespace." +) +@click.option( + "--out", + "-o", + "out_path", + type=click.Path(path_type=Path), + help="Newly created package directory. [default: SHORT_NAME]", +) def create(short_name: str, namespace: str, out_path: Path | None) -> None: + """Create new package.""" if not out_path: out_path = Path(short_name) if out_path.exists(): diff --git a/questionpy_sdk/commands/package.py b/questionpy_sdk/commands/package.py index 75e4c9cf..418809fa 100644 --- a/questionpy_sdk/commands/package.py +++ b/questionpy_sdk/commands/package.py @@ -2,19 +2,17 @@ # The QuestionPy SDK is free software released under terms of the MIT license. See LICENSE.md. # (c) Technische Universität Berlin, innoCampus -import inspect -import subprocess +import shutil +import tempfile +from contextlib import suppress from pathlib import Path -from tempfile import TemporaryDirectory -from types import ModuleType import click -import questionpy -from questionpy_sdk.commands._helper import create_normalized_filename, read_yaml_config -from questionpy_sdk.constants import PACKAGE_CONFIG_FILENAME -from questionpy_sdk.models import PackageConfig -from questionpy_sdk.package import PackageBuilder +from questionpy_sdk.package.builder import PackageBuilder +from questionpy_sdk.package.errors import PackageBuildError + +from ._helper import confirm_overwrite, read_package_source def validate_out_path(context: click.Context, _parameter: click.Parameter, value: Path | None) -> Path | None: @@ -26,54 +24,51 @@ def validate_out_path(context: click.Context, _parameter: click.Parameter, value @click.command() @click.argument("source", type=click.Path(exists=True, file_okay=False, path_type=Path)) -@click.option("--config", "-c", "config_path", type=click.Path(exists=True, dir_okay=False, path_type=Path)) -@click.option("--out", "-o", "out_path", callback=validate_out_path, type=click.Path(path_type=Path)) -def package(source: Path, config_path: Path | None, out_path: Path | None) -> None: - if not config_path: - config_path = source / PACKAGE_CONFIG_FILENAME - - config = read_yaml_config(config_path) +@click.option( + "--out", + "-o", + "out_path", + callback=validate_out_path, + type=click.Path(path_type=Path), + help="Output file path of QuestionPy package. [default: 'NAMESPACE-SHORT_NAME-VERSION.qpy']", +) +@click.option("--force", "-f", "force_overwrite", is_flag=True, help="Force overwriting of output file.") +@click.pass_context +def package(ctx: click.Context, source: Path, out_path: Path | None, *, force_overwrite: bool) -> None: + """Build package from directory SOURCE.""" + package_source = read_package_source(source) + overwriting = False if not out_path: - out_path = Path(create_normalized_filename(config)) - if out_path.exists() and click.confirm( - f"The path '{out_path}' already exists. Do you want to overwrite it?", abort=True - ): - out_path.unlink() + out_path = Path(package_source.normalized_filename) + + if out_path.exists(): + if force_overwrite or (not ctx.obj["no_interaction"] and confirm_overwrite(out_path)): + overwriting = True + else: + msg = f"Output file '{out_path}' exists" + raise click.ClickException(msg) try: - with PackageBuilder(out_path) as out_file: - _copy_package(out_file, questionpy) - _install_dependencies(out_file, config_path, config) - out_file.write_glob(source, "python/**/*") - out_file.write_glob(source, "static/**/*") - out_file.write_manifest(config) - except subprocess.CalledProcessError as e: - out_path.unlink(missing_ok=True) - msg = f"Failed to install requirements: {e.stderr.decode()}" - raise click.ClickException(msg) from e + # Use temp file, otherwise we risk overwriting `out_path` in case of a build error. + temp_file = tempfile.NamedTemporaryFile(delete=False) + temp_file_path = Path(temp_file.name) + + try: + with PackageBuilder(temp_file, package_source) as builder: + builder.write_package() + except PackageBuildError as exc: + msg = f"Failed to build package: {exc}" + raise click.ClickException(msg) from exc + finally: + temp_file.close() + + if overwriting: + Path(out_path).unlink() + + shutil.move(temp_file_path, out_path) + finally: + with suppress(FileNotFoundError): + temp_file_path.unlink() click.echo(f"Successfully created '{out_path}'.") - - -def _install_dependencies(target: PackageBuilder, config_path: Path, config: PackageConfig) -> None: - if isinstance(config.requirements, str): - # treat as a relative reference to a requirements.txt and read those - pip_args = ["-r", str(config_path.parent / config.requirements)] - elif isinstance(config.requirements, list): - # treat as individual dependency specifiers - pip_args = config.requirements - else: - # no dependencies specified - return - - with TemporaryDirectory(prefix=f"qpy_{config.short_name}") as tempdir: - subprocess.run(["pip", "install", "--target", tempdir, *pip_args], check=True, capture_output=True) # noqa: S603, S607 - target.write_glob(Path(tempdir), "**/*", prefix="dependencies/site-packages") - - -def _copy_package(target: PackageBuilder, pkg: ModuleType) -> None: - # inspect.getfile returns the path to the package's __init__.py - package_dir = Path(inspect.getfile(pkg)).parent - # TODO: Exclude __pycache__, pyc files and the like. - target.write_glob(package_dir, "**/*", prefix=f"dependencies/site-packages/{pkg.__name__}") diff --git a/questionpy_sdk/commands/repo/structure.py b/questionpy_sdk/commands/repo/structure.py index 21a61032..49b30b3b 100644 --- a/questionpy_sdk/commands/repo/structure.py +++ b/questionpy_sdk/commands/repo/structure.py @@ -7,8 +7,8 @@ import click -from questionpy_sdk.commands._helper import create_normalized_filename from questionpy_sdk.commands.repo._helper import IndexCreator, get_manifest +from questionpy_sdk.package._helper import create_normalized_filename @click.command() diff --git a/questionpy_sdk/package.py b/questionpy_sdk/package.py deleted file mode 100644 index 02597866..00000000 --- a/questionpy_sdk/package.py +++ /dev/null @@ -1,27 +0,0 @@ -# This file is part of the QuestionPy SDK. (https://questionpy.org) -# The QuestionPy SDK is free software released under terms of the MIT license. See LICENSE.md. -# (c) Technische Universität Berlin, innoCampus - -import logging -from pathlib import Path -from zipfile import ZipFile - -from questionpy_common.constants import MANIFEST_FILENAME -from questionpy_sdk.models import PackageConfig - -log = logging.getLogger(__name__) - - -class PackageBuilder(ZipFile): - def __init__(self, path: Path): - super().__init__(path, "w") - - def write_manifest(self, config: PackageConfig) -> None: - log.info("%s: %s", MANIFEST_FILENAME, config) - self.writestr(MANIFEST_FILENAME, config.manifest.model_dump_json()) - - def write_glob(self, source_dir: Path, glob: str, prefix: str = "") -> None: - for source_file in source_dir.glob(glob): - path_in_pkg = prefix / source_file.relative_to(source_dir) - log.info("%s: %s", path_in_pkg, source_file) - self.write(source_file, path_in_pkg) diff --git a/questionpy_sdk/package/__init__.py b/questionpy_sdk/package/__init__.py new file mode 100644 index 00000000..e750778c --- /dev/null +++ b/questionpy_sdk/package/__init__.py @@ -0,0 +1,3 @@ +# This file is part of the QuestionPy SDK. (https://questionpy.org) +# The QuestionPy SDK is free software released under terms of the MIT license. See LICENSE.md. +# (c) Technische Universität Berlin, innoCampus diff --git a/questionpy_sdk/package/_helper.py b/questionpy_sdk/package/_helper.py new file mode 100644 index 00000000..1ae14f10 --- /dev/null +++ b/questionpy_sdk/package/_helper.py @@ -0,0 +1,17 @@ +# This file is part of the QuestionPy SDK. (https://questionpy.org) +# The QuestionPy SDK is free software released under terms of the MIT license. See LICENSE.md. +# (c) Technische Universität Berlin, innoCampus + +from questionpy_common.manifest import Manifest + + +def create_normalized_filename(manifest: Manifest) -> str: + """Creates a normalized file name for the given manifest. + + Args: + manifest: manifest of the package + + Returns: + normalized file name + """ + return f"{manifest.namespace}-{manifest.short_name}-{manifest.version}.qpy" diff --git a/questionpy_sdk/package/builder.py b/questionpy_sdk/package/builder.py new file mode 100644 index 00000000..a006e043 --- /dev/null +++ b/questionpy_sdk/package/builder.py @@ -0,0 +1,130 @@ +# This file is part of the QuestionPy SDK. (https://questionpy.org) +# The QuestionPy SDK is free software released under terms of the MIT license. See LICENSE.md. +# (c) Technische Universität Berlin, innoCampus + +import inspect +import logging +import subprocess +from os import PathLike +from pathlib import Path +from tempfile import TemporaryDirectory +from zipfile import ZipFile + +import questionpy +from questionpy_common.constants import MANIFEST_FILENAME +from questionpy_sdk.models import BuildHookName +from questionpy_sdk.package.errors import PackageBuildError +from questionpy_sdk.package.source import PackageSource + +log = logging.getLogger(__name__) + + +class PackageBuilder(ZipFile): + """Builds `.qpy` packages. + + This class creates QuestionPy packages from a + [`PackageSource`][questionpy_sdk.package.source.PackageSource]. + """ + + def __init__(self, file: PathLike, source: PackageSource): + """Opens a package file for writing. + + Args: + file: Package output path. + source: Package source. + """ + super().__init__(file, "w") + self._source = source + + def write_package(self) -> None: + """Writes the `.qpy` package to the filesystem. + + Raises: + PackageBuildError: If the package failed to build. + """ + self._run_build_hooks("pre") + self._install_questionpy() + self._install_requirements() + self._write_package_files() + self._write_manifest() + self._run_build_hooks("post") + + def _run_build_hooks(self, hook_name: BuildHookName) -> None: + commands = self._source.config.build_hooks.get(hook_name, []) + + if isinstance(commands, str): + commands = [commands] + + for idx, cmd in enumerate(commands): + self._run_hook(cmd, hook_name, idx) + + def _install_questionpy(self) -> None: + """Adds the `questionpy` module to the package.""" + # getfile returns the path to the package's __init__.py + package_dir = Path(inspect.getfile(questionpy)).parent + # TODO: Exclude __pycache__, pyc files and the like. + self._write_glob(package_dir, "**/*.py", prefix=f"dependencies/site-packages/{questionpy.__name__}") + + def _install_requirements(self) -> None: + """Adds package requirements.""" + config = self._source.config + + # treat as relative reference to a requirements.txt and read those + if isinstance(config.requirements, str): + pip_args = ["-r", str(self._source.path / config.requirements)] + + # treat as individual dependency specifiers + elif isinstance(config.requirements, list): + pip_args = config.requirements + + # no dependencies specified + else: + return + + # pip doesn't offer a public API, so we have to resort to subprocess (pypa/pip#3121) + try: + with TemporaryDirectory(prefix=f"qpy_{config.short_name}") as tempdir: + subprocess.run(["pip", "install", "--target", tempdir, *pip_args], check=True, capture_output=True) # noqa: S603, S607 + self._write_glob(Path(tempdir), "**/*", prefix="dependencies/site-packages") + except subprocess.CalledProcessError as exc: + msg = f"Failed to install requirements: {exc.stderr.decode()}" + raise PackageBuildError(msg) from exc + + def _write_package_files(self) -> None: + """Writes custom package files.""" + self._write_glob(self._source.path, "python/**/*") + self._write_glob(self._source.path, "static/**/*") + self._write_glob(self._source.path, "js/**/*") + + def _write_manifest(self) -> None: + """Writes package manifest.""" + log.info("%s: %s", MANIFEST_FILENAME, self._source.config) + self.writestr(MANIFEST_FILENAME, self._source.config.manifest.model_dump_json()) + + def _write_glob(self, source_dir: Path, glob: str, prefix: str = "") -> None: + for source_file in source_dir.glob(glob): + path_in_pkg = prefix / source_file.relative_to(source_dir) + log.info("%s: %s", path_in_pkg, source_file) + self.write(source_file, path_in_pkg) + + def _run_hook(self, cmd: str, hook_name: BuildHookName, num: int) -> None: + log.info("Running %s hook[%d]: '%s'", hook_name, num, cmd) + proc = subprocess.Popen( + cmd, + cwd=self._source.path, + shell=True, # noqa: S602 + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + ) + if proc.stdout: + while True: + line = proc.stdout.readline() + if not line: + break + log.info("%s hook[%d]: %s", hook_name, num, line.rstrip()) + return_code = proc.wait() + if return_code != 0: + log.info("%s hook[%d] failed: '%s'", hook_name, num, cmd) + msg = f"{hook_name} hook[{num}] failed: '{cmd}'" + raise PackageBuildError(msg) diff --git a/questionpy_sdk/package/errors.py b/questionpy_sdk/package/errors.py new file mode 100644 index 00000000..4dc510d5 --- /dev/null +++ b/questionpy_sdk/package/errors.py @@ -0,0 +1,11 @@ +# This file is part of the QuestionPy SDK. (https://questionpy.org) +# The QuestionPy SDK is free software released under terms of the MIT license. See LICENSE.md. +# (c) Technische Universität Berlin, innoCampus + + +class PackageSourceValidationError(Exception): + pass + + +class PackageBuildError(Exception): + pass diff --git a/questionpy_sdk/package/source.py b/questionpy_sdk/package/source.py new file mode 100644 index 00000000..948299ef --- /dev/null +++ b/questionpy_sdk/package/source.py @@ -0,0 +1,78 @@ +# This file is part of the QuestionPy SDK. (https://questionpy.org) +# The QuestionPy SDK is free software released under terms of the MIT license. See LICENSE.md. +# (c) Technische Universität Berlin, innoCampus + +from pathlib import Path + +import yaml +from pydantic import ValidationError +from yaml import YAMLError + +from questionpy_sdk.constants import PACKAGE_CONFIG_FILENAME +from questionpy_sdk.models import PackageConfig +from questionpy_sdk.package.errors import PackageSourceValidationError + +from ._helper import create_normalized_filename + + +class PackageSource: + """Represents a package source directory on the filesystem.""" + + def __init__(self, path: Path): + """Initializes a package source from a directory path. + + Args: + path: Package source path. + + Raises: + PackageSourceValidationError: If the package source could not be validated. + """ + self._path = path + self._config = self._read_yaml_config() + self._validate() + + def _validate(self) -> None: + self._check_required_paths() + + def _check_required_paths(self) -> None: + # check for `python/NAMESPACE/SHORTNAME/__init__.py` + package_init_path = self._path / "python" / self._config.namespace / self._config.short_name / "__init__.py" + try: + package_init_path.stat() + except FileNotFoundError as exc: + msg = f"Expected '{package_init_path}' to exist" + raise PackageSourceValidationError(msg) from exc + if not package_init_path.is_file(): + msg = f"Expected '{package_init_path}' to be a file" + raise PackageSourceValidationError(msg) + + @property + def config(self) -> PackageConfig: + return self._config + + @property + def config_path(self) -> Path: + return self._path / PACKAGE_CONFIG_FILENAME + + @property + def normalized_filename(self) -> str: + return create_normalized_filename(self._config) + + @property + def path(self) -> Path: + return self._path + + def _read_yaml_config(self) -> PackageConfig: + try: + with self.config_path.open() as config_file: + return PackageConfig.model_validate(yaml.safe_load(config_file)) + except FileNotFoundError as exc: + msg = f"The config '{self.config_path}' does not exist." + raise PackageSourceValidationError(msg) from exc + except YAMLError as exc: + msg = f"Failed to parse config '{self.config_path}': {exc}" + raise PackageSourceValidationError(msg) from exc + except ValidationError as exc: + # TODO: pretty error feedback (https://docs.pydantic.dev/latest/errors/errors/#customize-error-messages) + msg = f"Failed to validate package config '{self.config_path}': {exc}" + raise PackageSourceValidationError(msg) from exc