From 95f348572c3eaf4f889826027436f9630763e240 Mon Sep 17 00:00:00 2001 From: jeanluc Date: Fri, 27 Sep 2024 21:42:36 +0200 Subject: [PATCH 1/2] Run all migrations in Python script * Adds migration helpers and dedicated script running in both before and after stages * Removes `copier_python` from context since it's already set by Copier in `_copier_python` and not available in `before` migrations for some reason * Makes the Copier helper independent of the current working directory, increasing robustness * Adds two new migrations and replaces the previous one, which depended on GNU CLI tools being available --- copier.yml | 19 ++++---- jinja_extensions/saltext.py | 2 - project/tools/helpers/copier.py | 50 ++++++++++++++++++++- tasks/migrations.py | 34 ++++++++++++++ tasks/task_helpers/migrate.py | 79 +++++++++++++++++++++++++++++++++ 5 files changed, 169 insertions(+), 15 deletions(-) create mode 100644 tasks/migrations.py create mode 100644 tasks/task_helpers/migrate.py diff --git a/copier.yml b/copier.yml index e994e34..0836423 100644 --- a/copier.yml +++ b/copier.yml @@ -364,7 +364,7 @@ max_salt_version_minor: type: int when: false default: >- - {%- if "." in (max_salt_version | string) -%} + {%- if "." in max_salt_version -%} {{- max_salt_version.split(".") | last -}} {%- else -%} {%- import "data/salt_latest_point.yaml" as spr -%} @@ -385,19 +385,16 @@ max_python_minor: # ======================================== _migrations: + # All general migrations are handled inside this script, which runs in both before and after stages. + - command: >- + "{{ _copier_python }}" + "{{ "{src}{sep}tasks{sep}migrations.py".format(src=_copier_conf.src_path, sep=_copier_conf.sep) }}" + when: 'true' # (Re)initialize the project and run pre-commit after updating - >- - "{{ copier_python }}" + "{{ _copier_python }}" "{{ "{src}{sep}tasks{sep}initialize.py".format(src=_copier_conf.src_path, sep=_copier_conf.sep) }}" migrate - - version: 0.3.0 - when: '{{ _stage == "before" }}' - # The minimum Python version was raised from 3.7 to 3.8. If the default answer - # is not changed before rendering the template, it fails. - command: >- - awk '/^python_requires:/ {sub("3.7", "3.8")}1' {{ _copier_conf.answers_file }} > - {{ _copier_conf.answers_file }}.tmp && - mv {{ _copier_conf.answers_file }}.tmp {{ _copier_conf.answers_file }} _tasks: # Don't run this when updating (updating copies the template twice). @@ -409,7 +406,7 @@ _tasks: not _copier_conf.answers.last and '.new_copy.' not in _copier_conf.dst_path.name and '.old_copy.' not in _copier_conf.dst_path.name -%} - "{{ copier_python }}" "{{ "{src}{sep}tasks{sep}initialize.py".format(src=_copier_conf.src_path, sep=_copier_conf.sep) }}" init + "{{ _copier_python }}" "{{ "{src}{sep}tasks{sep}initialize.py".format(src=_copier_conf.src_path, sep=_copier_conf.sep) }}" init {%- endif -%} # ===================================== diff --git a/jinja_extensions/saltext.py b/jinja_extensions/saltext.py index 1d69ca6..69a37af 100644 --- a/jinja_extensions/saltext.py +++ b/jinja_extensions/saltext.py @@ -1,5 +1,4 @@ import copy -import sys from pathlib import Path import yaml @@ -56,7 +55,6 @@ def hook(self, context): "salt_python_support": copy.deepcopy(self.sps), "singular_loader_dirs": SINGULAR_LOADER_DIRS, "salt_latest_point": copy.deepcopy(self.slp), - "copier_python": sys.executable, } diff --git a/project/tools/helpers/copier.py b/project/tools/helpers/copier.py index 91cf291..87ceba6 100644 --- a/project/tools/helpers/copier.py +++ b/project/tools/helpers/copier.py @@ -1,3 +1,4 @@ +import os import sys from functools import wraps from pathlib import Path @@ -13,7 +14,33 @@ yaml = None -COPIER_ANSWERS = Path(".copier-answers.yml").resolve() +if os.environ.get("STAGE"): + # If we're running inside a Copier task/migration, cwd is the target dir. + # We cannot use __file__ because this file is imported from the template clone. + COPIER_ANSWERS = Path(".copier-answers.yml").resolve() +else: + COPIER_ANSWERS = (Path(__file__).parent.parent.parent / ".copier-answers.yml").resolve() + + +if yaml is not None: + + def represent_str(dumper, data): + """ + Represent multiline strings using "|" + """ + if len(data.splitlines()) > 1: + return dumper.represent_scalar("tag:yaml.org,2002:str", data, style="|") + return dumper.represent_scalar("tag:yaml.org,2002:str", data) + + class OpinionatedYamlDumper(yaml.SafeDumper): + """ + Indent lists by two spaces + """ + + def increase_indent(self, flow=False, indentless=False): + return super().increase_indent(flow=flow, indentless=False) + + OpinionatedYamlDumper.add_representer(str, represent_str) def _needs_answers(func): @@ -33,10 +60,29 @@ def load_answers(): """ if not yaml: raise RuntimeError("Missing pyyaml in environment") - with open(COPIER_ANSWERS) as f: + with open(COPIER_ANSWERS, encoding="utf-8") as f: return yaml.safe_load(f) +@_needs_answers +def dump_answers(answers): + """ + Write the complete answers file. Depends on PyYAML. + Intended for answers migrations. + """ + if not yaml: + raise RuntimeError("Missing pyyaml in environment") + with open(COPIER_ANSWERS, "w", encoding="utf-8") as f: + yaml.dump( + answers, + f, + Dumper=OpinionatedYamlDumper, + indent=0, + default_flow_style=False, + canonical=False, + ) + + @_needs_answers def discover_project_name(): """ diff --git a/tasks/migrations.py b/tasks/migrations.py new file mode 100644 index 0000000..5ccbc89 --- /dev/null +++ b/tasks/migrations.py @@ -0,0 +1,34 @@ +from packaging.version import Version +from task_helpers.migrate import run_migrations +from task_helpers.migrate import var_migration + + +@var_migration("0.4.5", "max_salt_version") +def migrate_045_max_salt_version(val): + """ + Change max_salt_version from int to str. + """ + if not isinstance(val, str): + return str(val) + + +@var_migration("0.3.7", "docs_url") +def migrate_037_docs_url(val): + """ + If docs_url is empty, reset it to propose the new default. + """ + if not val: + return ... + + +@var_migration("0.3.0", "python_requires") +def migrate_030_python_requires(val): + """ + Raise minimum Python version to 3.8. + """ + if Version(val) < Version("3.8"): + return "3.8" + + +if __name__ == "__main__": + run_migrations() diff --git a/tasks/task_helpers/migrate.py b/tasks/task_helpers/migrate.py new file mode 100644 index 0000000..8f6d05d --- /dev/null +++ b/tasks/task_helpers/migrate.py @@ -0,0 +1,79 @@ +import os + +from packaging.version import Version + +from .pythonpath import project_tools + +with project_tools(): + from helpers.copier import dump_answers + from helpers.copier import load_answers + + +VERSION_FROM = Version(os.environ["VERSION_PEP440_FROM"]) +VERSION_TO = Version(os.environ["VERSION_PEP440_TO"]) +HEAD_UPDATE = bool(VERSION_TO.post) +STAGE = os.environ["STAGE"] + + +MIGRATIONS = [] + + +class Migration: + def __init__(self, func): + self.func = func + + def __call__(self, answers): + return self.func(answers) + + +class VarMigration(Migration): + def __init__(self, func, varname): + super().__init__(func) + self.varname = varname + + def __call__(self, answers): + if self.varname not in answers: + return + if (new_val := self.func(answers[self.varname])) is not None: + if new_val is ...: + answers.pop(self.varname) + else: + answers[self.varname] = new_val + return answers + + +def migration(trigger, stage="after"): + def wrapper(func): + if STAGE != stage: + return func + trigger_version = Version(trigger) + if VERSION_FROM >= trigger_version: + return func + # When updating with --vcs-ref=HEAD, run defined migrations independent of VERSION_TO. + # Otherwise, VERSION_TO should be at least the version trigger + if not (HEAD_UPDATE or trigger_version <= VERSION_TO): + return func + + if not isinstance(func, Migration): + func = Migration(func) + global MIGRATIONS + MIGRATIONS.append((trigger_version, func)) + return func + + return wrapper + + +def var_migration(trigger, varname): + def wrapper(func): + return migration(trigger, "before")(VarMigration(func, varname)) + + return wrapper + + +def run_migrations(): + if not MIGRATIONS: + return + answers = load_answers() + for _, func in sorted(MIGRATIONS): + func(answers) + dump_answers(answers) From 4699ddb7636073e74109c256741b0b324ecba27f Mon Sep 17 00:00:00 2001 From: jeanluc Date: Fri, 27 Sep 2024 23:22:21 +0200 Subject: [PATCH 2/2] Add migration status output, docstrings, changelog --- changelog/+pymigrations.fixed.md | 1 + tasks/task_helpers/migrate.py | 172 ++++++++++++++++++++++++------- 2 files changed, 137 insertions(+), 36 deletions(-) create mode 100644 changelog/+pymigrations.fixed.md diff --git a/changelog/+pymigrations.fixed.md b/changelog/+pymigrations.fixed.md new file mode 100644 index 0000000..4703397 --- /dev/null +++ b/changelog/+pymigrations.fixed.md @@ -0,0 +1 @@ +Moved migrations into Python script, making them more platform-agnostic diff --git a/tasks/task_helpers/migrate.py b/tasks/task_helpers/migrate.py index 8f6d05d..425050b 100644 --- a/tasks/task_helpers/migrate.py +++ b/tasks/task_helpers/migrate.py @@ -7,6 +7,7 @@ with project_tools(): from helpers.copier import dump_answers from helpers.copier import load_answers + from helpers.prompt import status VERSION_FROM = Version(os.environ["VERSION_PEP440_FROM"]) @@ -18,44 +19,96 @@ MIGRATIONS = [] -class Migration: - def __init__(self, func): - self.func = func +def run_migrations(): + """ + Run registered migrations valid for this update. + """ + if not MIGRATIONS: + return None + answers = load_answers() + if STAGE == "before": + return _run_migrations_before(answers) + return _run_migrations_after(answers) - def __call__(self, answers): - return self.func(answers) +def _run_migrations_before(answers): + """ + Run `before` migrations and dump the new answers. + """ + for _, func in sorted(MIGRATIONS): + ret = func(answers) + if ret is not None: + answers = ret + dump_answers(answers) -class VarMigration(Migration): - def __init__(self, func, varname): - super().__init__(func) - self.varname = varname - def __call__(self, answers): - if self.varname not in answers: - return - if (new_val := self.func(answers[self.varname])) is not None: - if new_val is ...: - answers.pop(self.varname) - else: - answers[self.varname] = new_val - return answers +def _run_migrations_after(answers): + """ + Run `after` migrations. + Answers can only be updated in the `before` stage. + """ + for _, func in sorted(MIGRATIONS): + func(answers.copy()) + + +def migration(trigger, stage="after", desc=None): + """ + Decorator for declaring a general migration. + + Decorated functions receive the complete answers dict as the sole parameter. + trigger + Version that triggers the migration when crossed during an update. + Set this to `None` to always trigger the migration. + + stage + Stage in which the migration should run, either `before` or `after`. + Defaults to `after`. + Set this to `None` to run the migration in both stages. + + desc + A description that is printed when running the migration. + Defaults to the decorated function's name with underscores + replaced by spaces. + + Examples: + + # Triggered when updating from a version below 1.0.0 to at least 1.0.0 + @migration("1.0.0") + def migrate_some_stuff_100(answers): + ... + + # Answers can only be changed in the `before` stage + @migration("1.0.0", stage="before") + def migrate_some_stuff_100(answers): + if answers["answer_a"] != "foo": + answers["answer_b"] = "bar" + + # This runs during all updates and in both stages + @migration(None, stage=None) + def do_some_voodoo(_): + ... + """ -def migration(trigger, stage="after"): def wrapper(func): - if STAGE != stage: - return func - trigger_version = Version(trigger) - if VERSION_FROM >= trigger_version: - return func - # When updating with --vcs-ref=HEAD, run defined migrations independent of VERSION_TO. - # Otherwise, VERSION_TO should be at least the version trigger - if not (HEAD_UPDATE or trigger_version <= VERSION_TO): + if stage is not None and STAGE != stage: return func - + if trigger is not None: + trigger_version = Version(trigger) + if VERSION_FROM >= trigger_version: + return func + # When updating with --vcs-ref=HEAD, run defined migrations independent of VERSION_TO. + # Otherwise, VERSION_TO should be at least the version trigger + if not (HEAD_UPDATE or trigger_version <= VERSION_TO): + return func + else: + # Run version-independent migrations last + trigger_version = Version("9999") + + # Other decorators delegate to this one, only create a general + # migration if it's not already another subtype if not isinstance(func, Migration): - func = Migration(func) + func = Migration(func, desc=desc or func.__name__.replace("_", " ")) global MIGRATIONS MIGRATIONS.append((trigger_version, func)) return func @@ -64,16 +117,63 @@ def wrapper(func): def var_migration(trigger, varname): + """ + Decorator for declaring an answer migration, e.g. when raising + a minimum version or changing an answer's type. + + Checks if the answer is defined in the answers file, if so + runs the decorated function with its value as the sole parameter. + + If the function returns an Ellipsis (...), resets the answer. + If the function returns a value other than None, updates the answer. + + trigger + Version that triggers the migration when crossed during an update. + + varname + The name of the answer that should be migrated. + + Example: + + @var_migration("1.0.0", "foo") + def migrate_foo_100(val): + if val < 4: + return 4 + """ + def wrapper(func): return migration(trigger, "before")(VarMigration(func, varname)) return wrapper -def run_migrations(): - if not MIGRATIONS: - return - answers = load_answers() - for _, func in sorted(MIGRATIONS): - func(answers) - dump_answers(answers) +class Migration: + def __init__(self, func, desc=None): + self.func = func + self.desc = desc + + def __call__(self, answers): + if self.desc is not None: + status(f"Running migration: {self.desc}") + return self.func(answers) + + +class VarMigration(Migration): + def __init__(self, func, varname): + super().__init__(func) + self.varname = varname + + def __call__(self, answers): + if self.varname not in answers: + return + if (new_val := self.func(answers[self.varname])) is not None: + if new_val is ...: + status(f"Answer migration: Resetting {self.varname}") + answers.pop(self.varname) + else: + status( + f"Answer migration: Updating {self.varname} from " + f"{answers[self.varname]!r} to {new_val!r}" + ) + answers[self.varname] = new_val + return answers