From 5ecb2704656ac720df438a6c08867a06df19fa2b Mon Sep 17 00:00:00 2001 From: Jean-Louis Fuchs Date: Wed, 13 Mar 2024 20:22:56 +0100 Subject: [PATCH] fix: get state_reader via function this is not much better, but the function name documents what is happening --- pyaptly/cli.py | 131 ++++++++++++++++++++++++++++++++++++++-- pyaptly/main.py | 58 ++++++++++-------- pyaptly/mirror.py | 8 +-- pyaptly/publish.py | 19 +++--- pyaptly/repo.py | 6 +- pyaptly/snapshot.py | 21 ++++--- pyaptly/state_reader.py | 9 ++- tools/rebuild-rpm | 1 + 8 files changed, 199 insertions(+), 54 deletions(-) diff --git a/pyaptly/cli.py b/pyaptly/cli.py index 2877677..769db5f 100644 --- a/pyaptly/cli.py +++ b/pyaptly/cli.py @@ -1,24 +1,53 @@ """python-click based command line interface for pyaptly.""" +import logging import sys from pathlib import Path +from subprocess import CalledProcessError import click -# I decided it is a good pattern to do lazy imports in the cli module. I had to -# do this in a few other CLIs for startup performance. +lg = logging.getLogger(__name__) -# TODO this makes the legacy command more usable. remove and set the entry point -# back to `pyaptly = 'pyaptly.cli:cli' def entry_point(): """Fix args then call click.""" + # TODO this makes the legacy command more usable. remove legacy commands when + # we are out of beta argv = list(sys.argv) len_argv = len(argv) if len_argv > 0 and argv[0].endswith("pyaptly"): if len_argv > 2 and argv[1] == "legacy" and argv[2] != "--": argv = argv[:2] + ["--"] + argv[2:] - cli.main(argv[1:]) + + try: + cli.main(argv[1:]) + except CalledProcessError: + pass # already logged + except Exception as e: + from . import util + + path = util.write_traceback() + tb = f"Wrote traceback to: {path}" + msg = " ".join([str(x) for x in e.args]) + lg.error(f"{msg}\n {tb}") + + +# I want to release the new cli interface with 2.0, so we do not repeat breaking changes. +# But changing all functions that use argparse, means also changing all the tests, which +# (ab)use the argparse interface. So we currently fake that interface, so we can roll-out +# the new interface early. +# TODO: remove this, once argparse is gone +class FakeArgs: + """Helper for compatiblity.""" + + def __init__(self, **kwargs): + for key, value in kwargs.items(): + setattr(self, key, value) + + +# I decided it is a good pattern to do lazy imports in the cli module. I had to +# do this in a few other CLIs for startup performance. @click.group() @@ -47,6 +76,98 @@ def legacy(passthrough): main.main(argv=passthrough) +@cli.command() +@click.option("--info/--no-info", "-i/-ni", default=False, type=bool) +@click.option("--debug/--no-debug", "-d/-nd", default=False, type=bool) +@click.option( + "--pretend/--no-pretend", + "-p/-np", + default=False, + type=bool, + help="Do not change anything", +) +@click.argument("config", type=click.Path(file_okay=True, dir_okay=False, exists=True)) +@click.argument("task", type=click.Choice(["create"])) +@click.option("--repo-name", "-n", default="all", type=str, help='deafult: "all"') +def repo(**kwargs): + """Create aptly repos.""" + from . import main, repo + + fake_args = FakeArgs(**kwargs) + main.setup_logger(fake_args) + cfg = main.prepare(fake_args) + repo.repo(cfg, args=fake_args) + + +@cli.command() +@click.option("--info/--no-info", "-i/-ni", default=False, type=bool) +@click.option("--debug/--no-debug", "-d/-nd", default=False, type=bool) +@click.option( + "--pretend/--no-pretend", + "-p/-np", + default=False, + type=bool, + help="Do not change anything", +) +@click.argument("config", type=click.Path(file_okay=True, dir_okay=False, exists=True)) +@click.argument("task", type=click.Choice(["create", "update"])) +@click.option("--mirror-name", "-n", default="all", type=str, help='deafult: "all"') +def mirror(**kwargs): + """Manage aptly mirrors.""" + from . import main, mirror + + fake_args = FakeArgs(**kwargs) + main.setup_logger(fake_args) + cfg = main.prepare(fake_args) + mirror.mirror(cfg, args=fake_args) + + +@cli.command() +@click.option("--info/--no-info", "-i/-ni", default=False, type=bool) +@click.option("--debug/--no-debug", "-d/-nd", default=False, type=bool) +@click.option( + "--pretend/--no-pretend", + "-p/-np", + default=False, + type=bool, + help="Do not change anything", +) +@click.argument("config", type=click.Path(file_okay=True, dir_okay=False, exists=True)) +@click.argument("task", type=click.Choice(["create", "update"])) +@click.option("--snapshot-name", "-n", default="all", type=str, help='deafult: "all"') +def snapshot(**kwargs): + """Manage aptly snapshots.""" + from . import main, snapshot + + fake_args = FakeArgs(**kwargs) + main.setup_logger(fake_args) + cfg = main.prepare(fake_args) + snapshot.snapshot(cfg, args=fake_args) + + +@cli.command() +@click.option("--info/--no-info", "-i/-ni", default=False, type=bool) +@click.option("--debug/--no-debug", "-d/-nd", default=False, type=bool) +@click.option( + "--pretend/--no-pretend", + "-p/-np", + default=False, + type=bool, + help="Do not change anything", +) +@click.argument("config", type=click.Path(file_okay=True, dir_okay=False, exists=True)) +@click.argument("task", type=click.Choice(["create", "update"])) +@click.option("--publish-name", "-n", default="all", type=str, help='deafult: "all"') +def publish(**kwargs): + """Manage aptly publishs.""" + from . import main, publish + + fake_args = FakeArgs(**kwargs) + main.setup_logger(fake_args) + cfg = main.prepare(fake_args) + publish.publish(cfg, args=fake_args) + + @cli.command(help="convert yaml- to toml-comfig") @click.argument( "yaml_path", diff --git a/pyaptly/main.py b/pyaptly/main.py index 853a3c7..03cf44d 100755 --- a/pyaptly/main.py +++ b/pyaptly/main.py @@ -22,13 +22,42 @@ lg = logging.getLogger(__name__) +def setup_logger(args): + """Setup the logger.""" + global _logging_setup + root = logging.getLogger() + formatter = custom_logger.CustomFormatter() + if not _logging_setup: # noqa + handler = logging.StreamHandler(sys.stderr) + handler.setFormatter(formatter) + root.addHandler(handler) + root.setLevel(logging.WARNING) + handler.setLevel(logging.WARNING) + if args.info: + root.setLevel(logging.INFO) + handler.setLevel(logging.INFO) + if args.debug: + root.setLevel(logging.DEBUG) + handler.setLevel(logging.DEBUG) + _logging_setup = True # noqa + + +def prepare(args): + """Set pretend mode, read config and load state.""" + command.Command.pretend_mode = args.pretend + + with open(args.config, "rb") as f: + cfg = tomli.load(f) + state_reader.state_reader().read() + return cfg + + def main(argv=None): """Define parsers and executes commands. :param argv: Arguments usually taken from sys.argv :type argv: list """ - global _logging_setup if not argv: # pragma: no cover argv = sys.argv[1:] parser = argparse.ArgumentParser(description="Manage aptly") @@ -78,31 +107,8 @@ def main(argv=None): repo_parser.add_argument("repo_name", type=str, nargs="?", default="all") args = parser.parse_args(argv) - root = logging.getLogger() - formatter = custom_logger.CustomFormatter() - if not _logging_setup: # noqa - handler = logging.StreamHandler(sys.stderr) - handler.setFormatter(formatter) - root.addHandler(handler) - root.setLevel(logging.WARNING) - handler.setLevel(logging.WARNING) - if args.info: - root.setLevel(logging.INFO) - handler.setLevel(logging.INFO) - if args.debug: - root.setLevel(logging.DEBUG) - handler.setLevel(logging.DEBUG) - if args.pretend: - command.Command.pretend_mode = True - else: - command.Command.pretend_mode = False - - _logging_setup = True # noqa - lg.debug("Args: %s", vars(args)) - - with open(args.config, "rb") as f: - cfg = tomli.load(f) - state_reader.state.read() + setup_logger(args) + cfg = prepare(args) # run function for selected subparser args.func(cfg, args) diff --git a/pyaptly/mirror.py b/pyaptly/mirror.py index 3cfc99e..26918a5 100644 --- a/pyaptly/mirror.py +++ b/pyaptly/mirror.py @@ -33,7 +33,7 @@ def add_gpg_keys(mirror_config): keys_urls[key] = None for key in keys_urls.keys(): - if key in state_reader.state.gpg_keys: + if key in state_reader.state_reader().gpg_keys: continue try: key_command = [ @@ -59,7 +59,7 @@ def add_gpg_keys(mirror_config): util.run_command(["bash", "-c", key_shell], check=True) else: raise - state_reader.state.read_gpg() + state_reader.state_reader().read_gpg() def mirror(cfg, args): @@ -102,7 +102,7 @@ def cmd_mirror_create(cfg, mirror_name, mirror_config): :param mirror_config: Configuration of the snapshot from the toml file. :type mirror_config: dict """ - if mirror_name in state_reader.state.mirrors: # pragma: no cover + if mirror_name in state_reader.state_reader().mirrors: # pragma: no cover return add_gpg_keys(mirror_config) @@ -142,7 +142,7 @@ def cmd_mirror_update(cfg, mirror_name, mirror_config): :param mirror_config: Configuration of the snapshot from the toml file. :type mirror_config: dict """ - if mirror_name not in state_reader.state.mirrors: # pragma: no cover + if mirror_name not in state_reader.state_reader().mirrors: # pragma: no cover raise Exception("Mirror not created yet") add_gpg_keys(mirror_config) aptly_cmd = ["aptly", "mirror", "update"] diff --git a/pyaptly/publish.py b/pyaptly/publish.py index ba2a3d5..0bd62a7 100644 --- a/pyaptly/publish.py +++ b/pyaptly/publish.py @@ -38,7 +38,7 @@ def publish(cfg, args): ] for cmd in command.Command.order_commands( - commands, state_reader.state.has_dependency + commands, state_reader.state_reader().has_dependency ): cmd.execute() @@ -49,7 +49,7 @@ def publish(cfg, args): for publish_conf_entry in cfg["publish"][args.publish_name] ] for cmd in command.Command.order_commands( - commands, state_reader.state.has_dependency + commands, state_reader.state_reader().has_dependency ): cmd.execute() else: @@ -83,7 +83,7 @@ def publish_cmd_update(cfg, publish_name, publish_config, ignore_existing=False) return command.Command(publish_cmd + options + args) publish_fullname = "%s %s" % (publish_name, publish_config["distribution"]) - current_snapshots = state_reader.state.publish_map[publish_fullname] + current_snapshots = state_reader.state_reader().publish_map[publish_fullname] if "snapshots" in publish_config: snapshots_config = publish_config["snapshots"] new_snapshots = [ @@ -97,7 +97,7 @@ def publish_cmd_update(cfg, publish_name, publish_config, ignore_existing=False) if publish["distribution"] == distribution: snapshots_config.extend(publish["snapshots"]) break - new_snapshots = list(state_reader.state.publish_map[conf_value]) + new_snapshots = list(state_reader.state_reader().publish_map[conf_value]) else: # pragma: no cover raise ValueError( "No snapshot references configured in publish %s" % publish_name @@ -121,7 +121,9 @@ def publish_cmd_update(cfg, publish_name, publish_config, ignore_existing=False) archive = archive.replace( "%T", date_tools.format_timestamp(datetime.datetime.now()) ) - if archive in state_reader.state.snapshots: # pragma: no cover + if ( + archive in state_reader.state_reader().snapshots + ): # pragma: no cover continue prefix_to_search = re.sub("%T$", "", snap["name"]) @@ -155,7 +157,10 @@ def publish_cmd_create(cfg, publish_name, publish_config, ignore_existing=False) :type publish_config: dict """ publish_fullname = "%s %s" % (publish_name, publish_config["distribution"]) - if publish_fullname in state_reader.state.publishes and not ignore_existing: + if ( + publish_fullname in state_reader.state_reader().publishes + and not ignore_existing + ): # Nothing to do, publish already created return @@ -228,7 +233,7 @@ def publish_cmd_create(cfg, publish_name, publish_config, ignore_existing=False) conf_value = " ".join(conf_value.split("/")) source_args.append("snapshot") try: - sources = state_reader.state.publish_map[conf_value] + sources = state_reader.state_reader().publish_map[conf_value] except KeyError: lg.critical( ( diff --git a/pyaptly/repo.py b/pyaptly/repo.py index 91fd851..d239ab2 100644 --- a/pyaptly/repo.py +++ b/pyaptly/repo.py @@ -30,7 +30,7 @@ def repo(cfg, args): ] for cmd in command.Command.order_commands( - commands, state_reader.state.has_dependency + commands, state_reader.state_reader().has_dependency ): cmd.execute() @@ -38,7 +38,7 @@ def repo(cfg, args): if args.repo_name in cfg["repo"]: commands = [cmd_repo(cfg, args.repo_name, cfg["repo"][args.repo_name])] for cmd in command.Command.order_commands( - commands, state_reader.state.has_dependency + commands, state_reader.state_reader().has_dependency ): cmd.execute() else: @@ -57,7 +57,7 @@ def repo_cmd_create(cfg, repo_name, repo_config): :param repo_config: Configuration of the repo from the toml file. :type repo_config: dict """ - if repo_name in state_reader.state.repos: # pragma: no cover + if repo_name in state_reader.state_reader().repos: # pragma: no cover # Nothing to do, repo already created return diff --git a/pyaptly/snapshot.py b/pyaptly/snapshot.py index e2259ef..d574037 100644 --- a/pyaptly/snapshot.py +++ b/pyaptly/snapshot.py @@ -43,7 +43,7 @@ def snapshot(cfg, args): if len(commands) > 0: for cmd in command.Command.order_commands( - commands, state_reader.state.has_dependency + commands, state_reader.state_reader().has_dependency ): cmd.execute() @@ -55,7 +55,7 @@ def snapshot(cfg, args): if len(commands) > 0: for cmd in command.Command.order_commands( - commands, state_reader.state.has_dependency + commands, state_reader.state_reader().has_dependency ): cmd.execute() @@ -115,7 +115,9 @@ def dependents_of_snapshot(snapshot_name): :rtype: generator """ - for dependent in state_reader.state.snapshot_map.get(snapshot_name, []): + for dependent in state_reader.state_reader().snapshot_map.get( + snapshot_name, [] + ): yield dependent # TODO I fixed a bug, but there is no test. We do not test recursive dependants yield from dependents_of_snapshot(dependent) @@ -140,7 +142,7 @@ def rotate_snapshot(cfg, snapshot_name): # First, verify that our snapshot environment is in a sane state_reader.state. # Fixing the environment is not currently our task. - if rotated_name in state_reader.state.snapshots: # pragma: no cover + if rotated_name in state_reader.state_reader().snapshots: # pragma: no cover raise Exception( "Cannot update snapshot %s - rotated name %s already exists" % (snapshot_name, rotated_name) @@ -185,7 +187,7 @@ def cmd_snapshot_update( # The "intermediate" command causes the state reader to refresh. At the # same time, it provides a collection point for dependency handling. - intermediate = command.FunctionCommand(state_reader.state.read) + intermediate = command.FunctionCommand(state_reader.state_reader().read) intermediate.provide("virtual", "all-snapshots-rotated") for cmd in rename_cmds: @@ -200,7 +202,7 @@ def cmd_snapshot_update( # Same as before - create a focal point to "collect" dependencies # after the snapshots have been rebuilt. Also reload state once again - intermediate2 = command.FunctionCommand(state_reader.state.read) + intermediate2 = command.FunctionCommand(state_reader.state_reader().read) intermediate2.provide("virtual", "all-snapshots-rebuilt") create_cmds = [] @@ -245,7 +247,7 @@ def cmd_snapshot_update( def is_publish_affected(name, publish_info): if ( "%s %s" % (name, publish_info["distribution"]) - in state_reader.state.publishes + in state_reader.state_reader().publishes ): try: for snap in publish_info["snapshots"]: @@ -316,7 +318,10 @@ def cmd_snapshot_create( snapshot_name = date_tools.expand_timestamped_name(snapshot_name, snapshot_config) - if snapshot_name in state_reader.state.snapshots and not ignore_existing: + if ( + snapshot_name in state_reader.state_reader().snapshots + and not ignore_existing + ): return [] default_aptly_cmd = ["aptly", "snapshot", "create"] diff --git a/pyaptly/state_reader.py b/pyaptly/state_reader.py index 779ae0f..f4a66c2 100644 --- a/pyaptly/state_reader.py +++ b/pyaptly/state_reader.py @@ -175,4 +175,11 @@ def has_dependency(self, dependency): raise ValueError("Unknown dependency to resolve: %s" % str(dependency)) -state = SystemStateReader() +_state_reader: SystemStateReader | None = None + + +def state_reader(): + global _state_reader + if not _state_reader: + _state_reader = SystemStateReader() + return _state_reader diff --git a/tools/rebuild-rpm b/tools/rebuild-rpm index 75fdce0..95e5b73 100755 --- a/tools/rebuild-rpm +++ b/tools/rebuild-rpm @@ -18,6 +18,7 @@ def main(): "python3-tomli-w", "python3-devel", "python3-setuptools", + "python3-colorama", ], check=True, )