From df979583aae3e441b00c3f0d650a894073cdb5d1 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 20 Oct 2026 01:06:57 +0400 Subject: [PATCH 01/24] Add the releases.json record and its publication checks to the interface `microcosm_emitter_interface.releases` holds the one record of which build of each release is current, and the rules every write follows: - Shape: schema_version 1, `default`, and for each release its `role`, `current` (a build or null) and each build's `published_at` and `withdrawn`; serialized in that order with releases and builds sorted. - Changes: `add_build` (optionally making it current), `make_current` and `withdraw_build`. A listed build cannot be published again, a release keeps its first role, a withdrawn build cannot become current and the current build cannot be withdrawn. - Checks: `check_record` (current names a listed, non-withdrawn build), `check_change` (existing build entries change only `withdrawn`), `check_repository` (every listed build has its folder and a release_manifest.json naming the same release and build) and `check_commit` (no file inside an existing build folder is added, changed or removed; no latest*.json is written; current does not change while any latest*.json exists). Co-Authored-By: Claude Opus 5.5 --- changelog.d/releases-record-checks.added.md | 1 + .../microcosm_emitter_interface/releases.py | 436 ++++++++++++++++++ tests/interface/test_releases_record.py | 368 +++++++++++++++ 3 files changed, 805 insertions(+) create mode 100644 changelog.d/releases-record-checks.added.md create mode 100644 packages/microcosm-emitter-interface/src/microcosm_emitter_interface/releases.py create mode 100644 tests/interface/test_releases_record.py diff --git a/changelog.d/releases-record-checks.added.md b/changelog.d/releases-record-checks.added.md new file mode 100644 index 00000000..8d0ba82c --- /dev/null +++ b/changelog.d/releases-record-checks.added.md @@ -0,0 +1 @@ +Add `microcosm_emitter_interface.releases`: the shape of each dataset repository's `releases.json`, the changes the release writer makes (publish a build, make a build current, withdraw a build), and the checks every write must pass, so Microcosm and the emitter run the same code. diff --git a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/releases.py b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/releases.py new file mode 100644 index 00000000..232e5330 --- /dev/null +++ b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/releases.py @@ -0,0 +1,436 @@ +"""``releases.json``: the only record of which build of each release is current. + +Each dataset repository publishes builds side by side, never changed after +publishing:: + + releases.json + releases///release_manifest.json + releases///... + +and one ``releases.json`` at its root names, for each release, its role, its +current build and every published build. This module holds the record's +shape, the three changes the release writer makes (publish a build, make a +build current, withdraw a build), and the checks every write must pass. The +emitter's release writer runs these checks before each commit; Microcosm and +a scheduled repository check can run the same code. + +Not settled yet, and kept out of these rules on purpose: whether a release +such as the dense one has a current build (``current`` may be ``null``), who +may promote or withdraw (the writer's caller decides), how readers show +withdrawn builds, and whether the US layout moves to the same scheme. Nothing +here depends on a country. +""" + +from __future__ import annotations + +import fnmatch +import json +from collections.abc import Callable, Collection, Mapping +from dataclasses import dataclass, field, replace +from typing import Any, Final + +from microcosm_emitter_interface.identifiers import role_identifier, safe_identifier + +RELEASES_RECORD_PATH: Final = "releases.json" +RELEASES_SCHEMA_VERSION: Final = 1 +RELEASES_FOLDER: Final = "releases" +RELEASE_MANIFEST_NAME: Final = "release_manifest.json" +#: Pointer files the record replaces (``latest.json``, ``latest-national.json``, +#: ``latest-evidence.json``, ...). No write may create one, and ``current`` +#: may not change while any still exists. +POINTER_FILE_PATTERN: Final = "latest*.json" + +_RECORD_KEYS: Final = {"schema_version", "default", "releases"} +_RELEASE_KEYS: Final = {"role", "current", "builds"} +_BUILD_KEYS: Final = {"published_at", "withdrawn"} + + +@dataclass(frozen=True) +class Violation: + """One broken rule: a fixed ``code`` and a message without secrets.""" + + code: str + message: str + + +class ReleaseRecordError(ValueError): + """A record or a change that breaks the publication rules.""" + + def __init__(self, violations: list[Violation]) -> None: + self.violations = list(violations) + super().__init__("; ".join(violation.message for violation in violations)) + + +def _refuse(code: str, message: str) -> ReleaseRecordError: + return ReleaseRecordError([Violation(code, message)]) + + +@dataclass(frozen=True) +class BuildEntry: + published_at: str + withdrawn: bool = False + + +@dataclass(frozen=True) +class ReleaseEntry: + role: str + current: str | None + builds: Mapping[str, BuildEntry] = field(default_factory=dict) + + +@dataclass(frozen=True) +class ReleasesRecord: + """A parsed, structurally valid ``releases.json``.""" + + default: str | None + releases: Mapping[str, ReleaseEntry] + + @classmethod + def empty(cls) -> ReleasesRecord: + return cls(default=None, releases={}) + + @classmethod + def parse(cls, content: bytes) -> ReleasesRecord: + try: + document = json.loads(content) + except (ValueError, UnicodeDecodeError): + raise _refuse("INVALID_RECORD", "releases.json is not JSON.") from None + record = cls._from_document(document) + violations = check_record(record) + if violations: + raise ReleaseRecordError(violations) + return record + + @classmethod + def _from_document(cls, document: Any) -> ReleasesRecord: + def invalid(message: str) -> ReleaseRecordError: + return _refuse("INVALID_RECORD", message) + + if not isinstance(document, dict) or set(document) != _RECORD_KEYS: + raise invalid(f"releases.json must have exactly {sorted(_RECORD_KEYS)}.") + if document["schema_version"] != RELEASES_SCHEMA_VERSION: + raise invalid( + f"releases.json schema_version must be {RELEASES_SCHEMA_VERSION}." + ) + releases_document = document["releases"] + if not isinstance(releases_document, dict): + raise invalid("releases must be an object.") + releases = {} + for release, entry in releases_document.items(): + try: + safe_identifier(release, label="release") + except ValueError: + raise invalid("A release name is not a safe identifier.") from None + if not isinstance(entry, dict) or set(entry) != _RELEASE_KEYS: + raise invalid(f"Release {release} must have {sorted(_RELEASE_KEYS)}.") + try: + role_identifier(entry["role"], label="release role") + except ValueError: + raise invalid(f"Release {release} has an invalid role.") from None + if entry["current"] is not None and not isinstance(entry["current"], str): + raise invalid(f"Release {release} current must be a build or null.") + if not isinstance(entry["builds"], dict): + raise invalid(f"Release {release} builds must be an object.") + builds = {} + for build, build_entry in entry["builds"].items(): + try: + safe_identifier(build, label="build") + except ValueError: + raise invalid("A build name is not a safe identifier.") from None + if ( + not isinstance(build_entry, dict) + or set(build_entry) != _BUILD_KEYS + or not isinstance(build_entry["published_at"], str) + or not build_entry["published_at"] + or not isinstance(build_entry["withdrawn"], bool) + ): + raise invalid(f"Build {build} of {release} is malformed.") + builds[build] = BuildEntry( + build_entry["published_at"], build_entry["withdrawn"] + ) + releases[release] = ReleaseEntry(entry["role"], entry["current"], builds) + default = document["default"] + if default is not None and not isinstance(default, str): + raise invalid("default must be a release name or null.") + return cls(default=default, releases=releases) + + def to_document(self) -> dict[str, Any]: + return { + "schema_version": RELEASES_SCHEMA_VERSION, + "default": self.default, + "releases": { + release: { + "role": entry.role, + "current": entry.current, + "builds": { + build: { + "published_at": build_entry.published_at, + "withdrawn": build_entry.withdrawn, + } + for build, build_entry in sorted(entry.builds.items()) + }, + } + for release, entry in sorted(self.releases.items()) + }, + } + + def serialize(self) -> bytes: + return (json.dumps(self.to_document(), indent=2) + "\n").encode() + + +def build_folder(release: str, build: str) -> str: + return f"{RELEASES_FOLDER}/{release}/{build}" + + +def is_pointer_file(path: str) -> bool: + return fnmatch.fnmatchcase(path.rsplit("/", 1)[-1], POINTER_FILE_PATTERN) + + +# Changes ----------------------------------------------------------------- + + +def add_build( + record: ReleasesRecord, + *, + release: str, + build: str, + release_role: str, + published_at: str, + make_current: bool, +) -> ReleasesRecord: + """List a newly published build; optionally make it the release's current.""" + safe_identifier(release, label="release") + safe_identifier(build, label="build") + role_identifier(release_role, label="release role") + existing = record.releases.get(release) + if existing is not None and existing.role != release_role: + raise _refuse( + "RELEASE_ROLE_DIFFERS", + f"Release {release} has role {existing.role}, not {release_role}.", + ) + entry = existing or ReleaseEntry(role=release_role, current=None, builds={}) + if build in entry.builds: + raise _refuse("BUILD_ALREADY_LISTED", f"Build {build} of {release} is listed.") + builds = {**entry.builds, build: BuildEntry(published_at, False)} + updated = replace( + entry, builds=builds, current=build if make_current else entry.current + ) + return replace(record, releases={**record.releases, release: updated}) + + +def make_current(record: ReleasesRecord, *, release: str, build: str) -> ReleasesRecord: + """Promote a listed, non-withdrawn build.""" + entry = record.releases.get(release) + if entry is None or build not in entry.builds: + raise _refuse("BUILD_NOT_LISTED", f"Build {build} of {release} is not listed.") + if entry.builds[build].withdrawn: + raise _refuse( + "CURRENT_WITHDRAWN", f"Build {build} of {release} has been withdrawn." + ) + return replace( + record, + releases={**record.releases, release: replace(entry, current=build)}, + ) + + +def withdraw_build( + record: ReleasesRecord, *, release: str, build: str +) -> ReleasesRecord: + """Mark a listed build withdrawn; its folder and files stay. + + The current build cannot be withdrawn: make another build current first. + """ + entry = record.releases.get(release) + if entry is None or build not in entry.builds: + raise _refuse("BUILD_NOT_LISTED", f"Build {build} of {release} is not listed.") + if entry.current == build: + raise _refuse( + "WITHDRAWING_CURRENT_BUILD", + f"Build {build} is the current build of {release}.", + ) + builds = {**entry.builds, build: replace(entry.builds[build], withdrawn=True)} + return replace( + record, releases={**record.releases, release: replace(entry, builds=builds)} + ) + + +# Checks ------------------------------------------------------------------ + + +def check_record(record: ReleasesRecord) -> list[Violation]: + """``current`` names a listed, non-withdrawn build; ``default`` a release.""" + violations = [] + if record.default is not None and record.default not in record.releases: + violations.append( + Violation("DEFAULT_NOT_LISTED", f"Default {record.default} is not listed.") + ) + for release, entry in record.releases.items(): + if entry.current is None: + continue + build_entry = entry.builds.get(entry.current) + if build_entry is None: + violations.append( + Violation( + "CURRENT_NOT_LISTED", + f"Current build {entry.current} of {release} is not listed.", + ) + ) + elif build_entry.withdrawn: + violations.append( + Violation( + "CURRENT_WITHDRAWN", + f"Current build {entry.current} of {release} is withdrawn.", + ) + ) + return violations + + +def check_change( + before: ReleasesRecord | None, after: ReleasesRecord +) -> list[Violation]: + """Existing build entries are unchanged except for ``withdrawn``.""" + violations = check_record(after) + if before is None: + return violations + for release, entry in before.releases.items(): + updated = after.releases.get(release) + if updated is None: + violations.append( + Violation("RELEASE_REMOVED", f"Release {release} was removed.") + ) + continue + for build, build_entry in entry.builds.items(): + updated_build = updated.builds.get(build) + if updated_build is None: + violations.append( + Violation( + "BUILD_ENTRY_REMOVED", + f"Build {build} of {release} was removed.", + ) + ) + elif updated_build.published_at != build_entry.published_at: + violations.append( + Violation( + "BUILD_ENTRY_CHANGED", + f"Build {build} of {release} changed beyond withdrawn.", + ) + ) + return violations + + +def check_repository( + record: ReleasesRecord, + files: Collection[str], + read_file: Callable[[str], bytes], +) -> list[Violation]: + """Every listed build has its folder and a manifest naming it. + + ``files`` lists the repository's paths; ``read_file`` returns a path's + bytes, so the caller decides where they come from (a pinned revision, or + the local files about to be committed). + """ + violations = [] + paths = set(files) + for release, entry in sorted(record.releases.items()): + for build in sorted(entry.builds): + folder = build_folder(release, build) + if not any(path.startswith(folder + "/") for path in paths): + violations.append( + Violation("BUILD_FOLDER_MISSING", f"{folder}/ holds no files.") + ) + continue + manifest_path = f"{folder}/{RELEASE_MANIFEST_NAME}" + if manifest_path not in paths: + violations.append( + Violation("MANIFEST_MISSING", f"{manifest_path} is missing.") + ) + continue + try: + manifest = json.loads(read_file(manifest_path)) + except (ValueError, UnicodeDecodeError): + manifest = None + if ( + not isinstance(manifest, dict) + or manifest.get("release") != release + or manifest.get("build") != build + ): + violations.append( + Violation( + "MANIFEST_MISMATCH", + f"{manifest_path} does not name release {release} " + f"and build {build}.", + ) + ) + return violations + + +def _build_folder_of(path: str) -> str | None: + parts = path.split("/") + if len(parts) >= 4 and parts[0] == RELEASES_FOLDER: + return "/".join(parts[:3]) + return None + + +def _current_builds(record: ReleasesRecord | None) -> dict[str, str | None]: + if record is None: + return {} + return {release: entry.current for release, entry in record.releases.items()} + + +def check_commit( + *, + before_files: Collection[str], + added_paths: Collection[str], + deleted_paths: Collection[str], + before: ReleasesRecord | None, + after: ReleasesRecord, +) -> list[Violation]: + """Rules on what one commit writes, given the repository it is built on. + + - No file inside an existing build folder is added, changed or removed, + whether or not ``releases.json`` lists that build. + - No write creates a ``latest*.json`` pointer file. + - ``current`` does not change while any ``latest*.json`` file exists: + until the old pointer files are deleted, they and the record could + disagree. + """ + violations = [] + existing_folders = { + folder + for folder in (_build_folder_of(path) for path in before_files) + if folder is not None + } + if before is not None: + existing_folders |= { + build_folder(release, build) + for release, entry in before.releases.items() + for build in entry.builds + } + for path in sorted({*added_paths, *deleted_paths}): + if _build_folder_of(path) in existing_folders: + violations.append( + Violation( + "EXISTING_BUILD_FOLDER_CHANGED", + f"{path} is inside an existing build folder.", + ) + ) + for path in sorted(added_paths): + if is_pointer_file(path): + violations.append( + Violation("POINTER_FILE_WRITE", f"{path} is a pointer file.") + ) + before_current = _current_builds(before) + changed_current = any( + before_current.get(release) != current + for release, current in _current_builds(after).items() + ) or any(release not in after.releases for release in before_current) + pointer_files = sorted(path for path in before_files if is_pointer_file(path)) + if changed_current and pointer_files: + violations.append( + Violation( + "POINTER_FILES_PRESENT", + "current cannot change while pointer files exist: " + + ", ".join(pointer_files), + ) + ) + return violations diff --git a/tests/interface/test_releases_record.py b/tests/interface/test_releases_record.py new file mode 100644 index 00000000..c7afc13d --- /dev/null +++ b/tests/interface/test_releases_record.py @@ -0,0 +1,368 @@ +"""The releases.json record and the publication checks Microcosm shares.""" + +import json + +import pytest + +from microcosm_emitter_interface.releases import ( + ReleaseRecordError, + ReleasesRecord, + add_build, + build_folder, + check_change, + check_commit, + check_record, + check_repository, + make_current, + withdraw_build, +) + +NATIONAL = "microcosm-uk-2024-25-national" +DENSE = "microcosm-uk-2024-25-dense" +OLDER_BUILD = "uk-frs-calibration-attempt-20261014T214443Z-d12ff9b3" +NEWER_BUILD = "uk-frs-calibration-attempt-20261019T083323Z-6cba5d71" + +DOCUMENTED_EXAMPLE = { + "schema_version": 1, + "default": NATIONAL, + "releases": { + NATIONAL: { + "role": "national_default", + "current": NEWER_BUILD, + "builds": { + OLDER_BUILD: { + "published_at": "2026-10-14T21:44:43+00:00", + "withdrawn": False, + }, + NEWER_BUILD: { + "published_at": "2026-10-19T08:33:23+00:00", + "withdrawn": False, + }, + }, + } + }, +} + + +def documented_record() -> ReleasesRecord: + return ReleasesRecord.parse(json.dumps(DOCUMENTED_EXAMPLE).encode()) + + +def codes(violations) -> set[str]: + return {violation.code for violation in violations} + + +def manifest(release: str, build: str) -> bytes: + return json.dumps({"release": release, "build": build}).encode() + + +def repository_files(record: ReleasesRecord) -> dict[str, bytes]: + files = {} + for release, entry in record.releases.items(): + for build in entry.builds: + folder = build_folder(release, build) + files[f"{folder}/release_manifest.json"] = manifest(release, build) + files[f"{folder}/microcosm_uk_2024_25.h5"] = b"rows" + return files + + +def test_documented_record_round_trips_in_its_documented_shape(): + record = documented_record() + document = json.loads(record.serialize()) + assert document == DOCUMENTED_EXAMPLE + assert list(document) == ["schema_version", "default", "releases"] + assert list(document["releases"][NATIONAL]) == ["role", "current", "builds"] + assert record.serialize().endswith(b"}\n") + assert check_record(record) == [] + + +def test_an_empty_record_has_no_default_and_no_releases(): + assert json.loads(ReleasesRecord.empty().serialize()) == { + "schema_version": 1, + "default": None, + "releases": {}, + } + + +@pytest.mark.parametrize( + "edit", + [ + lambda document: document.update(schema_version=2), + lambda document: document.update(default="unknown-release"), + lambda document: document.update(extra=True), + lambda document: document["releases"][NATIONAL].update(current="unlisted"), + lambda document: document["releases"][NATIONAL]["builds"][NEWER_BUILD].update( + withdrawn=True + ), + lambda document: document["releases"][NATIONAL]["builds"][OLDER_BUILD].update( + withdrawn="no" + ), + lambda document: document["releases"][NATIONAL]["builds"][OLDER_BUILD].update( + published_at=5 + ), + lambda document: document["releases"][NATIONAL]["builds"][OLDER_BUILD].update( + note="extra" + ), + lambda document: document["releases"].update({"../escape": {}}), + lambda document: document["releases"][NATIONAL].pop("role"), + ], + ids=[ + "schema-version", + "default-not-listed", + "extra-key", + "current-not-listed", + "current-withdrawn", + "withdrawn-not-boolean", + "published-at-not-text", + "extra-build-key", + "release-identifier", + "missing-role", + ], +) +def test_records_that_break_the_shape_are_refused(edit): + document = json.loads(json.dumps(DOCUMENTED_EXAMPLE)) + edit(document) + with pytest.raises(ReleaseRecordError): + ReleasesRecord.parse(json.dumps(document).encode()) + + +def test_publishing_adds_a_build_and_can_make_it_current(): + later = "uk-frs-calibration-attempt-20261020T000000Z-0000aaaa" + record = add_build( + documented_record(), + release=NATIONAL, + build=later, + release_role="national_default", + published_at="2026-10-20T00:00:00+00:00", + make_current=True, + ) + assert record.releases[NATIONAL].current == later + assert record.releases[NATIONAL].builds[later].withdrawn is False + assert check_change(documented_record(), record) == [] + + +def test_a_build_can_be_published_without_becoming_current(): + record = add_build( + documented_record(), + release=DENSE, + build=NEWER_BUILD, + release_role="local_area_dense", + published_at="2026-10-20T00:00:00+00:00", + make_current=False, + ) + assert record.releases[DENSE].current is None + assert record.releases[DENSE].role == "local_area_dense" + assert record.releases[NATIONAL] == documented_record().releases[NATIONAL] + assert check_record(record) == [] + + +def test_publishing_a_listed_build_again_is_refused(): + with pytest.raises(ReleaseRecordError) as refusal: + add_build( + documented_record(), + release=NATIONAL, + build=OLDER_BUILD, + release_role="national_default", + published_at="2026-10-20T00:00:00+00:00", + make_current=False, + ) + assert codes(refusal.value.violations) == {"BUILD_ALREADY_LISTED"} + + +def test_a_release_keeps_the_role_it_was_first_published_with(): + with pytest.raises(ReleaseRecordError) as refusal: + add_build( + documented_record(), + release=NATIONAL, + build="another-build", + release_role="local_area", + published_at="2026-10-20T00:00:00+00:00", + make_current=False, + ) + assert codes(refusal.value.violations) == {"RELEASE_ROLE_DIFFERS"} + + +def test_promoting_names_a_listed_build_that_is_not_withdrawn(): + record = make_current(documented_record(), release=NATIONAL, build=OLDER_BUILD) + assert record.releases[NATIONAL].current == OLDER_BUILD + with pytest.raises(ReleaseRecordError) as unlisted: + make_current(documented_record(), release=NATIONAL, build="unlisted") + assert codes(unlisted.value.violations) == {"BUILD_NOT_LISTED"} + withdrawn = withdraw_build(documented_record(), release=NATIONAL, build=OLDER_BUILD) + with pytest.raises(ReleaseRecordError) as refusal: + make_current(withdrawn, release=NATIONAL, build=OLDER_BUILD) + assert codes(refusal.value.violations) == {"CURRENT_WITHDRAWN"} + + +def test_withdrawing_sets_only_withdrawn_and_refuses_the_current_build(): + before = documented_record() + after = withdraw_build(before, release=NATIONAL, build=OLDER_BUILD) + assert after.releases[NATIONAL].builds[OLDER_BUILD].withdrawn is True + assert ( + after.releases[NATIONAL].builds[OLDER_BUILD].published_at + == before.releases[NATIONAL].builds[OLDER_BUILD].published_at + ) + assert check_change(before, after) == [] + with pytest.raises(ReleaseRecordError) as refusal: + withdraw_build(before, release=NATIONAL, build=NEWER_BUILD) + assert codes(refusal.value.violations) == {"WITHDRAWING_CURRENT_BUILD"} + + +def test_existing_build_entries_may_change_only_their_withdrawn_flag(): + before = documented_record() + changed = json.loads(json.dumps(DOCUMENTED_EXAMPLE)) + changed["releases"][NATIONAL]["builds"][OLDER_BUILD]["published_at"] = "later" + assert codes( + check_change(before, ReleasesRecord.parse(json.dumps(changed).encode())) + ) == {"BUILD_ENTRY_CHANGED"} + removed = json.loads(json.dumps(DOCUMENTED_EXAMPLE)) + del removed["releases"][NATIONAL]["builds"][OLDER_BUILD] + assert codes( + check_change(before, ReleasesRecord.parse(json.dumps(removed).encode())) + ) == {"BUILD_ENTRY_REMOVED"} + gone = {"schema_version": 1, "default": None, "releases": {}} + assert codes( + check_change(before, ReleasesRecord.parse(json.dumps(gone).encode())) + ) == {"RELEASE_REMOVED"} + assert check_change(None, before) == [] + + +def test_every_listed_build_needs_its_folder_and_a_manifest_naming_it(): + record = documented_record() + files = repository_files(record) + assert check_repository(record, files, files.__getitem__) == [] + newer_folder = build_folder(NATIONAL, NEWER_BUILD) + without_folder = { + path: content + for path, content in files.items() + if not path.startswith(newer_folder + "/") + } + assert codes( + check_repository(record, without_folder, without_folder.__getitem__) + ) == {"BUILD_FOLDER_MISSING"} + without_manifest = dict(files) + del without_manifest[f"{newer_folder}/release_manifest.json"] + assert codes( + check_repository(record, without_manifest, without_manifest.__getitem__) + ) == {"MANIFEST_MISSING"} + for wrong in (manifest(NATIONAL, OLDER_BUILD), manifest(DENSE, NEWER_BUILD), b"["): + mismatched = {**files, f"{newer_folder}/release_manifest.json": wrong} + assert codes(check_repository(record, mismatched, mismatched.__getitem__)) == { + "MANIFEST_MISMATCH" + } + + +def test_no_file_inside_an_existing_build_folder_may_be_added_or_removed(): + record = documented_record() + files = repository_files(record) + new_folder = build_folder(NATIONAL, "uk-frs-calibration-attempt-new") + assert ( + check_commit( + before_files=files, + added_paths=[f"{new_folder}/release_manifest.json", "releases.json"], + deleted_paths=[], + before=record, + after=record, + ) + == [] + ) + listed_folder = build_folder(NATIONAL, OLDER_BUILD) + assert codes( + check_commit( + before_files=files, + added_paths=[f"{listed_folder}/extra.json"], + deleted_paths=[], + before=record, + after=record, + ) + ) == {"EXISTING_BUILD_FOLDER_CHANGED"} + assert codes( + check_commit( + before_files=files, + added_paths=[], + deleted_paths=[f"{listed_folder}/microcosm_uk_2024_25.h5"], + before=record, + after=record, + ) + ) == {"EXISTING_BUILD_FOLDER_CHANGED"} + unlisted_folder = build_folder(NATIONAL, "copied-but-unlisted") + with_unlisted = {**files, f"{unlisted_folder}/release_manifest.json": b"{}"} + assert codes( + check_commit( + before_files=with_unlisted, + added_paths=[f"{unlisted_folder}/microcosm_uk_2024_25.h5"], + deleted_paths=[], + before=record, + after=record, + ) + ) == {"EXISTING_BUILD_FOLDER_CHANGED"} + + +@pytest.mark.parametrize( + "pointer_path", + ["latest.json", "latest-national.json", "latest-evidence.json", "x/latest_y.json"], +) +def test_no_write_may_create_a_pointer_file(pointer_path): + record = documented_record() + assert codes( + check_commit( + before_files=repository_files(record), + added_paths=[pointer_path], + deleted_paths=[], + before=record, + after=record, + ) + ) == {"POINTER_FILE_WRITE"} + + +def test_current_cannot_change_while_pointer_files_exist(): + before = documented_record() + files = {**repository_files(before), "latest-national.json": b"{}"} + promoted = make_current(before, release=NATIONAL, build=OLDER_BUILD) + assert codes( + check_commit( + before_files=files, + added_paths=["releases.json"], + deleted_paths=[], + before=before, + after=promoted, + ) + ) == {"POINTER_FILES_PRESENT"} + published = add_build( + before, + release=NATIONAL, + build="uk-frs-calibration-attempt-new", + release_role="national_default", + published_at="2026-10-20T00:00:00+00:00", + make_current=False, + ) + assert ( + check_commit( + before_files=files, + added_paths=[ + f"{build_folder(NATIONAL, 'uk-frs-calibration-attempt-new')}/a.json", + "releases.json", + ], + deleted_paths=[], + before=before, + after=published, + ) + == [] + ) + first_record_with_current = add_build( + ReleasesRecord.empty(), + release=NATIONAL, + build=NEWER_BUILD, + release_role="national_default", + published_at="2026-10-20T00:00:00+00:00", + make_current=True, + ) + assert codes( + check_commit( + before_files={"latest.json": b"{}"}, + added_paths=["releases.json"], + deleted_paths=[], + before=None, + after=first_record_with_current, + ) + ) == {"POINTER_FILES_PRESENT"} From d211eda43ccdd7c2eed8977acb0bf2e755ad41a1 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 20 Oct 2026 01:13:38 +0400 Subject: [PATCH 02/24] Publish release builds side by side through the release writer The emitter's `huggingface` module now owns every write to a dataset repository's releases.json (nothing calls it yet; Microcosm switches in a later change): - `PublishRelease` names the release, the build, whether to make it current, the release's role and each file with its role; it must include release_manifest.json. `PromoteBuild` and `WithdrawBuild` change only releases.json. - Each change reads releases.json and the file list at main's head, applies the change, runs the shared checks from microcosm_emitter_interface.releases and commits the new build folder and releases.json with parent_commit. On a parent-commit conflict it reads the new head and applies the change again. - No Hugging Face tags or branches, no current/ folder, no latest*.json; existing tags and files are untouched. release.py's staging branch, tag and pointer-file commit are replaced, not ported. - A retried publication that already landed reports published; preserved files are re-hashed; Hub failures stay pending and are retried. - `release_change_policy` is the extension point for who may promote. - Jobs no longer wait for the 10-second spacing, which applies to the frequent run-file commits only. - Protocol fixtures for the three release messages. Co-Authored-By: Claude Opus 5.5 --- README.md | 15 + changelog.d/release-writer.added.md | 1 + .../microcosm-emitter-interface/README.md | 3 + .../microcosm_emitter_interface/__init__.py | 10 + .../src/microcosm_emitter_interface/client.py | 47 ++ .../huggingface-promote-build.json | 16 + .../huggingface-publish-release.json | 46 ++ .../huggingface-withdraw-build.json | 16 + .../publication.py | 221 ++++++++++ .../huggingface/service/delivery.py | 93 ++-- .../huggingface/service/module.py | 67 ++- .../huggingface/service/queue.py | 9 +- .../huggingface/service/releases.py | 255 +++++++++++ .../huggingface/service/requests.py | 78 ++++ tests/huggingface/test_release_writer.py | 405 ++++++++++++++++++ tests/interface/test_client_messages.py | 48 +++ tests/interface/test_protocol_fixtures.py | 9 + tests/interface/test_release_client.py | 177 ++++++++ 18 files changed, 1468 insertions(+), 48 deletions(-) create mode 100644 changelog.d/release-writer.added.md create mode 100644 packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-promote-build.json create mode 100644 packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-publish-release.json create mode 100644 packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-withdraw-build.json create mode 100644 packages/microcosm-emitter-interface/src/microcosm_emitter_interface/publication.py create mode 100644 src/microcosm_emitter/huggingface/service/releases.py create mode 100644 tests/huggingface/test_release_writer.py create mode 100644 tests/interface/test_release_client.py diff --git a/README.md b/README.md index a1342d7d..39baa9ee 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,21 @@ The `huggingface` module takes over the uploads Microcosm made itself. Microcosm **Bundle uploads run on their own thread.** A bundle upload can take minutes, so the module uploads bundles on a worker thread of its own, while the host's worker thread commits run files and makes the commit at shutdown. A bundle upload to one repository therefore never delays run-file commits to another repository, the commit at shutdown, or a read-back. Once shutdown begins, the bundle worker starts no further upload, even to a repository left in its current pass: it checks for shutdown before each repository, again once it has leased the repository, and again once it has claimed a job, immediately before that upload begins, and it gives back a job claimed after shutdown began without attempting it. So during the commit at shutdown a repository can be held only by an upload that passed that last check before shutdown began, or, for the few database operations it takes to give it back, by a lease the worker took just before; the second commit at shutdown covers that repository if the worker stops within the shutdown period. Each delivery leases its repository first, so this machine never commits to one repository twice at once: run files for the repository a bundle is uploading to wait for that upload, and then for the 10-second spacing. +**Releases.** The release writer replaces the commit, tag and pointer-file writes of Microcosm's `release.py`. Nothing calls it yet; Microcosm switches to it in a later change. Each build is published side by side and never changed: + +``` +releases.json the only record of what is current +releases///release_manifest.json +releases///... the build's files, with their roles +``` + +- `publish_release` names the release, the build (Microcosm's attempt ID), whether to make the build current, the release's role, and each file with its role. `promote_build` changes a release's current build; `withdraw_build` marks a build withdrawn, and its folder stays. +- Every change is one commit: the writer reads `releases.json` and the file list at `main`'s head, applies the change, runs the shared checks in `microcosm_emitter_interface.releases`, and commits the new build folder and `releases.json` with `parent_commit` set to that head. If another commit lands first, it reads the new head and applies the change again, so concurrent publications keep each other's builds. +- The writer refuses a change unless: every listed build has its folder and a `release_manifest.json` naming the same release and build; `current` names a listed build that is not withdrawn; existing build entries change only `withdrawn`; and no file inside an existing build folder is added, changed or removed (so publishing refuses a build whose folder exists). It never writes a `latest*.json` file, and refuses to change `current` while any `latest*.json` file still exists, until the old pointer files are deleted. +- It creates no Hugging Face tags or branches, no `current/` folder and no mutable root file, and leaves existing tags and files untouched. It does not port `release.py`'s staging branch, release tag or pointer-file commit, which this layout replaces. +- A retried publication whose earlier attempt already landed reports `published`. Hub failures stay `pending` and are retried; broken rules are `refused` with the rule's code. +- `release_change_policy` is where a decision on who may promote or withdraw will plug in; by default every change is allowed. `current` may stay `null`, for a release that is never promoted. + ## Development Use Python 3.13 or 3.14 on Linux or macOS. diff --git a/changelog.d/release-writer.added.md b/changelog.d/release-writer.added.md new file mode 100644 index 00000000..9f3e464e --- /dev/null +++ b/changelog.d/release-writer.added.md @@ -0,0 +1 @@ +Add the release writer and the interface's `publish_release`, `promote_build`, `withdraw_build` and `release_receipt` calls. Each build is written to `releases///` with an updated `releases.json` in one commit pinned to `main`'s head, re-applied after a parent-commit conflict, and checked against the shared rules: no new tags, no `latest*.json` files, no change to an existing build folder, and no change of `current` while any `latest*.json` file exists. Nothing calls the writer yet. diff --git a/packages/microcosm-emitter-interface/README.md b/packages/microcosm-emitter-interface/README.md index 6c4f0274..b5dd2b55 100644 --- a/packages/microcosm-emitter-interface/README.md +++ b/packages/microcosm-emitter-interface/README.md @@ -32,8 +32,11 @@ No call raises into the build: not because the emitter is missing, slow or refus | Telemetry event | `client.telemetry.stage(...)`, `transition_stage`, `progress`, `calibration_progress`, `fail`, `refuse`, `complete`, `emit` (each one `emit_telemetry`) | `telemetry` | | Staging run files under `runs//` | `stage_run_files(StageRunFiles(...))`, `run_files_receipt`, `read_back_run_files` | `huggingface` | | Staged dataset bundle, each file with its role | `stage_dataset_bundle(StageDatasetBundle(...))`, `dataset_bundle_receipt`, `check_write_access` | `huggingface` | +| Release publication: release, build, whether current, files with roles | `publish_release(PublishRelease(...))`, `promote_build`, `withdraw_build`, `release_receipt` | `huggingface` | | Orrery inputs | `orrery_inventory`, `hand_over_orrery_inputs`, `orrery_receipt`, `retry_orrery_inputs` | `orrery` | +`microcosm_emitter_interface.releases` holds the shape of `releases.json` and the checks every release write must pass, so Microcosm, the emitter and a scheduled repository check run the same code. + ## Compatibility Both packages are released together with one version number, and `microcosm-emitter==X` requires `microcosm-emitter-interface==X`. A build, though, may run with a different emitter from the interface it depends on: it keeps the emitter it started with, and the next build uses whichever emitter is installed then. So the socket protocol carries its own version: diff --git a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/__init__.py b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/__init__.py index 4583df39..bb2a727b 100644 --- a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/__init__.py +++ b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/__init__.py @@ -12,6 +12,12 @@ ) from microcosm_emitter_interface.orrery import OrrerySettings from microcosm_emitter_interface.protocol import PROTOCOL_VERSION +from microcosm_emitter_interface.publication import ( + PromoteBuild, + PublishRelease, + ReleaseFile, + WithdrawBuild, +) from microcosm_emitter_interface.spool import ( default_handover_database_path, default_spool_path, @@ -33,11 +39,15 @@ "HuggingFaceSettings", "OrrerySettings", "RequestFailure", + "PromoteBuild", + "PublishRelease", + "ReleaseFile", "RunFile", "StageDatasetBundle", "StageRunFiles", "TelemetryRun", "TelemetrySettings", + "WithdrawBuild", "default_handover_database_path", "default_spool_path", ] diff --git a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/client.py b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/client.py index 04d6d56d..c90161ce 100644 --- a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/client.py +++ b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/client.py @@ -58,6 +58,14 @@ STARTUP_TIMEOUT_SECONDS, TELEMETRY_MODULE, ) +from microcosm_emitter_interface.publication import ( + PromoteBuild, + PublishRelease, + ReleaseChange, + ReleaseReceipt, + WithdrawBuild, + unavailable_release_receipt, +) from microcosm_emitter_interface.spool import default_spool_path from microcosm_emitter_interface.staging import ( HuggingFaceSettings, @@ -486,6 +494,45 @@ def check_write_access(self, repository: str) -> WriteAccess: return cast(WriteAccess, value) return unavailable_write_access() + def _release_change( + self, request: ReleaseChange, wait_seconds: float + ) -> ReleaseReceipt: + message = request.message() + response = self._query( + HUGGING_FACE_MODULE, + lambda: message, + kind="release", + timeout=FILE_REQUEST_TIMEOUT_SECONDS, + ) + value = None if response is None else response.get("receipt") + if not isinstance(value, dict): + return unavailable_release_receipt(request) + return cast(ReleaseReceipt, self._wait_until_settled(value, wait_seconds)) + + def publish_release( + self, request: PublishRelease, *, wait_seconds: float = 300.0 + ) -> ReleaseReceipt: + """Queue a build's publication and wait for it to land or be refused. + + The emitter writes ``releases///`` and ``releases.json`` + in one commit. After ``wait_seconds`` a ``pending`` receipt means the + emitter is still retrying; ask again with ``release_receipt``. + """ + return self._release_change(request, wait_seconds) + + def promote_build( + self, request: PromoteBuild, *, wait_seconds: float = 60.0 + ) -> ReleaseReceipt: + return self._release_change(request, wait_seconds) + + def withdraw_build( + self, request: WithdrawBuild, *, wait_seconds: float = 60.0 + ) -> ReleaseReceipt: + return self._release_change(request, wait_seconds) + + def release_receipt(self, handover_id: str) -> ReleaseReceipt | None: + return cast(ReleaseReceipt | None, self._handover_receipt(handover_id)) + def close(self) -> None: """Ask the emitter to drain and stop; repeated calls do nothing.""" if self._closed: diff --git a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-promote-build.json b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-promote-build.json new file mode 100644 index 00000000..e2eb6760 --- /dev/null +++ b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-promote-build.json @@ -0,0 +1,16 @@ +{ + "protocol_version": 1, + "module": "huggingface", + "kind": "query", + "sequence": 9, + "body": { + "protocol_version": 1, + "command": "promote_build", + "repository": "policyengine/fixture-private", + "release": "fixture-release", + "build": "fixture-build" + }, + "response_keys": [ + "receipt" + ] +} diff --git a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-publish-release.json b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-publish-release.json new file mode 100644 index 00000000..0ea59b4c --- /dev/null +++ b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-publish-release.json @@ -0,0 +1,46 @@ +{ + "protocol_version": 1, + "module": "huggingface", + "kind": "query", + "sequence": 8, + "files": { + "release_manifest.json": "{\"release\":\"fixture-release\",\"build\":\"fixture-build\"}", + "dataset.h5": "synthetic rows", + "calibration_diagnostics.json": "{\"schema_version\":8}" + }, + "body": { + "protocol_version": 1, + "command": "publish_release", + "repository": "policyengine/fixture-private", + "release": "fixture-release", + "build": "fixture-build", + "release_role": "national_default", + "make_current": true, + "files": [ + { + "name": "release_manifest.json", + "role": "release_manifest", + "local_path": "{directory}/release_manifest.json", + "sha256": "9f82c735620892c3f85f288cb9f3e46f5d811c53d5da1745c80aade58b161c85", + "bytes": 53 + }, + { + "name": "dataset.h5", + "role": "dataset", + "local_path": "{directory}/dataset.h5", + "sha256": "5cc590aa15e97595e13963c72aea8d3fbe65fbf5dd5f8fc690ee479f73d45548", + "bytes": 14 + }, + { + "name": "calibration_diagnostics.json", + "role": "calibration_diagnostics", + "local_path": "{directory}/calibration_diagnostics.json", + "sha256": "2300b80de59210bc04c40b1aade68e7b5fc265901dbc9af4e9f506c0bcc36e7c", + "bytes": 20 + } + ] + }, + "response_keys": [ + "receipt" + ] +} diff --git a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-withdraw-build.json b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-withdraw-build.json new file mode 100644 index 00000000..e85f68fe --- /dev/null +++ b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-withdraw-build.json @@ -0,0 +1,16 @@ +{ + "protocol_version": 1, + "module": "huggingface", + "kind": "query", + "sequence": 10, + "body": { + "protocol_version": 1, + "command": "withdraw_build", + "repository": "policyengine/fixture-private", + "release": "fixture-release", + "build": "fixture-build" + }, + "response_keys": [ + "receipt" + ] +} diff --git a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/publication.py b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/publication.py new file mode 100644 index 00000000..f7eaf1f6 --- /dev/null +++ b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/publication.py @@ -0,0 +1,221 @@ +"""Typed release publication: one build folder and one ``releases.json`` change. + +``PublishRelease`` names the release, the build (the attempt ID Microcosm +already assigns), whether to make the build current, and each file with its +role. The emitter writes the files to ``releases///`` and the +updated ``releases.json`` in one commit, after the checks in +``microcosm_emitter_interface.releases``. ``PromoteBuild`` and +``WithdrawBuild`` change only ``releases.json``. + +Publication is not best-effort: the receipt says whether the change was +``published``, is still ``pending`` (retried from the emitter's queue) or was +``refused`` by a rule. Constructing a request that breaks the shape rules +raises ``ValueError``. +""" + +from __future__ import annotations + +import re +from collections.abc import Sequence +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any, Final, Literal, TypedDict + +from microcosm_emitter_interface.bundles import handover_identifier +from microcosm_emitter_interface.identifiers import ( + bare_file_name, + repository_identifier, + role_identifier, + safe_identifier, +) +from microcosm_emitter_interface.protocol import PROTOCOL_VERSION +from microcosm_emitter_interface.releases import ( + RELEASE_MANIFEST_NAME, + is_pointer_file, +) + +RELEASE_MANIFEST_ROLE: Final = "release_manifest" +RELEASE_CHANGE: Final = "release_change" +_SHA256: Final = re.compile(r"^[0-9a-f]{64}$") + + +@dataclass(frozen=True) +class ReleaseFile: + """One preserved file of the build and what it is (``dataset``, ...).""" + + name: str + role: str + local_path: Path + sha256: str + bytes: int + + def __post_init__(self) -> None: + bare_file_name(self.name, label="release file name") + role_identifier(self.role) + if is_pointer_file(self.name): + raise ValueError("A release file may not be a latest*.json pointer.") + if not isinstance(self.sha256, str) or not _SHA256.fullmatch(self.sha256): + raise ValueError("sha256 must be a lowercase hexadecimal digest.") + if ( + isinstance(self.bytes, bool) + or not isinstance(self.bytes, int) + or self.bytes < 0 + ): + raise ValueError("bytes must be a nonnegative integer.") + + +def _check_reference(repository: str, release: str, build: str) -> None: + repository_identifier(repository) + safe_identifier(release, label="release") + safe_identifier(build, label="build") + + +@dataclass(frozen=True) +class PublishRelease: + """Publish a new build of a release, optionally as its current build. + + ``release_role`` is the release's role in ``releases.json`` (for example + ``national_default``); a release keeps the role it was first published + with. The files must include ``release_manifest.json`` with the role + ``release_manifest``, naming this release and build. + """ + + repository: str + release: str + build: str + release_role: str + make_current: bool + files: Sequence[ReleaseFile] + protocol_version: int = field(default=PROTOCOL_VERSION, repr=False) + + def __post_init__(self) -> None: + _check_reference(self.repository, self.release, self.build) + role_identifier(self.release_role, label="release_role") + if not isinstance(self.make_current, bool): + raise ValueError("make_current must be a boolean.") + object.__setattr__(self, "files", tuple(self.files)) + if any(not isinstance(item, ReleaseFile) for item in self.files): + raise ValueError("files must be ReleaseFile values.") + names = [item.name for item in self.files] + if len(set(names)) != len(names): + raise ValueError("A build names each file once.") + manifests = [item for item in self.files if item.name == RELEASE_MANIFEST_NAME] + if len(manifests) != 1 or manifests[0].role != RELEASE_MANIFEST_ROLE: + raise ValueError( + f"A build has one {RELEASE_MANIFEST_NAME} with the role " + f"{RELEASE_MANIFEST_ROLE}." + ) + if [item.role for item in self.files].count(RELEASE_MANIFEST_ROLE) != 1: + raise ValueError(f"Only {RELEASE_MANIFEST_NAME} has its role.") + + def message(self) -> dict[str, Any]: + return { + "protocol_version": self.protocol_version, + "command": "publish_release", + "repository": self.repository, + "release": self.release, + "build": self.build, + "release_role": self.release_role, + "make_current": self.make_current, + "files": [ + { + "name": item.name, + "role": item.role, + "local_path": str(Path(item.local_path).resolve()), + "sha256": item.sha256, + "bytes": item.bytes, + } + for item in self.files + ], + } + + def handover_id(self) -> str: + return handover_identifier(RELEASE_CHANGE, self.message()) + + +@dataclass(frozen=True) +class PromoteBuild: + """Make a listed, non-withdrawn build the release's current build.""" + + repository: str + release: str + build: str + protocol_version: int = field(default=PROTOCOL_VERSION, repr=False) + + def __post_init__(self) -> None: + _check_reference(self.repository, self.release, self.build) + + def message(self) -> dict[str, Any]: + return { + "protocol_version": self.protocol_version, + "command": "promote_build", + "repository": self.repository, + "release": self.release, + "build": self.build, + } + + def handover_id(self) -> str: + return handover_identifier(RELEASE_CHANGE, self.message()) + + +@dataclass(frozen=True) +class WithdrawBuild: + """Mark a build withdrawn; its folder and files stay as published.""" + + repository: str + release: str + build: str + protocol_version: int = field(default=PROTOCOL_VERSION, repr=False) + + def __post_init__(self) -> None: + _check_reference(self.repository, self.release, self.build) + + def message(self) -> dict[str, Any]: + return { + "protocol_version": self.protocol_version, + "command": "withdraw_build", + "repository": self.repository, + "release": self.release, + "build": self.build, + } + + def handover_id(self) -> str: + return handover_identifier(RELEASE_CHANGE, self.message()) + + +type ReleaseChange = PublishRelease | PromoteBuild | WithdrawBuild + + +class ReleaseReceipt(TypedDict): + """The emitter's latest result for one release change. + + ``published`` and ``refused`` are final; ``pending`` is retried, with the + last attempt's ``error_code`` (``REPOSITORY_UNAVAILABLE``, + ``UPLOAD_FAILED``, ``PARENT_CONFLICT``, ``CREDENTIAL_MISSING``, + ``PRESERVED_FILES_MISSING``). A refusal's ``error_code`` names the broken + rule, for example ``BUILD_FOLDER_EXISTS`` or ``POINTER_FILES_PRESENT``. + """ + + handover_id: str + status: Literal["pending", "published", "refused", "unavailable"] + error_code: str | None + message: str | None + commit: str | None + repository: str + release: str + build: str + attempts: int + + +def unavailable_release_receipt(request: ReleaseChange) -> ReleaseReceipt: + return { + "handover_id": request.handover_id(), + "status": "unavailable", + "error_code": None, + "message": None, + "commit": None, + "repository": request.repository, + "release": request.release, + "build": request.build, + "attempts": 0, + } diff --git a/src/microcosm_emitter/huggingface/service/delivery.py b/src/microcosm_emitter/huggingface/service/delivery.py index 7cc6c0f4..0d1c1e29 100644 --- a/src/microcosm_emitter/huggingface/service/delivery.py +++ b/src/microcosm_emitter/huggingface/service/delivery.py @@ -1,11 +1,12 @@ """Delivery cycles: at most one Hugging Face commit per repository per cycle. Each cycle finds the repositories with work due, leases each one, and makes -one commit: a due job (a dataset bundle), or every pending staging run file of -that repository at once. The module runs the run-file cycle on the host's -thread and each job kind's cycle on its own worker thread (see ``workers``), -so a long upload holds only its own repository. Failures keep the work queued -and back off; nothing here raises into the build. +one commit: a due job (a dataset bundle or a release change), or every +pending staging run file of that repository at once. The module runs the +run-file cycle on the host's thread and each job kind's cycle on its own +worker thread (see ``workers``), so a long upload holds only its own +repository. Failures keep the work queued and back off; nothing here raises +into the build. """ from __future__ import annotations @@ -21,6 +22,11 @@ from microcosm_emitter.huggingface.service.bundles import stage_bundle from microcosm_emitter.huggingface.service.hub import DatasetRepository from microcosm_emitter.huggingface.service.queue import ClaimedJob, HandoverQueue +from microcosm_emitter.huggingface.service.releases import ( + ReleaseChangePolicy, + allow_every_change, + apply_release_change, +) from microcosm_emitter.huggingface.service.run_files import commit_run_files #: How long a repository or job lease lasts without renewal. While a delivery @@ -31,7 +37,8 @@ LEASE_RENEWAL_SECONDS = 30.0 #: The job kinds this emitter delivers. DATASET_BUNDLE = "dataset_bundle" -SUPPORTED_JOB_KINDS = frozenset({DATASET_BUNDLE}) +RELEASE_CHANGE = "release_change" +SUPPORTED_JOB_KINDS = frozenset({DATASET_BUNDLE, RELEASE_CHANGE}) CREDENTIAL_MISSING = "CREDENTIAL_MISSING" UPLOAD_FAILED = "UPLOAD_FAILED" CREDENTIAL_MISSING_MESSAGE = ( @@ -131,11 +138,13 @@ def __init__( api_factory: Callable[[], Any | None], clock: Callable[[], float] = time.time, lease_renewal_seconds: float = LEASE_RENEWAL_SECONDS, + release_change_policy: ReleaseChangePolicy = allow_every_change, ) -> None: self.queue = queue self.api_factory = api_factory self.clock = clock self.lease_renewal_seconds = lease_renewal_seconds + self.release_change_policy = release_change_policy self._warned: set[str] = set() self._warning_lock = threading.Lock() @@ -285,17 +294,9 @@ def _run_files_to_commit( ) def _claim_job(self, repository: str, kinds: frozenset[str]) -> ClaimedJob | None: - """The next due job of ``kinds``, once the commit spacing has elapsed. - - Checked here, after the repository is leased: another thread may have - committed to it since this cycle listed it. - """ - now = self.clock() - if not self.queue.commit_spacing_elapsed(repository, now): - return None return self.queue.claim_job( repository, - now=now, + now=self.clock(), lease_seconds=REPOSITORY_LEASE_SECONDS, kinds=kinds, ) @@ -306,30 +307,49 @@ def _finish_job( *, status: str, error_code: str | None, - revision: str | None, retry: bool, committed: bool = False, + revision: str | None = None, + message: str | None = None, ) -> None: + receipt: dict[str, Any] = { + "handover_id": job.handover_id, + "status": status, + "error_code": error_code, + "attempts": job.attempts + 1, + "repository": job.target, + } + if job.kind == RELEASE_CHANGE: + receipt.update( + message=message, + commit=revision, + release=job.request["release"], + build=job.request["build"], + ) + else: + receipt.update( + revision=revision, + prefix=f"{job.request['prefix']}/{job.request['run_id']}", + copied_files=sorted(job.files), + ) self.queue.finish_job( - job, - receipt={ - "handover_id": job.handover_id, - "status": status, - "error_code": error_code, - "revision": revision, - "attempts": job.attempts + 1, - "repository": job.target, - "prefix": f"{job.request['prefix']}/{job.request['run_id']}", - "copied_files": sorted(job.files), - }, - retry=retry, - now=self.clock(), - committed=committed, + job, receipt=receipt, retry=retry, now=self.clock(), committed=committed ) def _deliver_job(self, hub: DatasetRepository, held: HeldLeases) -> bool: - assert held.job is not None, "The job is claimed before delivery." - outcome = stage_bundle(hub, held.job.request, held.job.files) + job = held.job + assert job is not None, "The job is claimed before delivery." + if job.kind == RELEASE_CHANGE: + outcome = apply_release_change( + hub, + job.request, + published_at=iso_timestamp(self.clock()), + policy=self.release_change_policy, + ) + revision, message = outcome.commit, outcome.message + else: + outcome = stage_bundle(hub, job.request, job.files) + revision, message = outcome.revision, None if outcome.retry and outcome.error_code is not None: self.warn_once( outcome.error_code, @@ -341,9 +361,10 @@ def _deliver_job(self, hub: DatasetRepository, held: HeldLeases) -> bool: held.job, status=outcome.status, error_code=outcome.error_code, - revision=outcome.revision, retry=outcome.retry, committed=outcome.committed, + revision=revision, + message=message, ) return outcome.committed @@ -363,11 +384,7 @@ def _credential_missing( held.stop() assert held.job is not None self._finish_job( - held.job, - status="pending", - error_code=CREDENTIAL_MISSING, - revision=None, - retry=True, + held.job, status="pending", error_code=CREDENTIAL_MISSING, retry=True ) return if not self._run_files_to_commit(repository, run_files, at_shutdown): diff --git a/src/microcosm_emitter/huggingface/service/module.py b/src/microcosm_emitter/huggingface/service/module.py index 48c6fb86..df234a76 100644 --- a/src/microcosm_emitter/huggingface/service/module.py +++ b/src/microcosm_emitter/huggingface/service/module.py @@ -5,8 +5,9 @@ - staging run-file commits, and the commit at shutdown, on the host's worker thread (``tick`` and ``close``); -- dataset bundle uploads, which can take minutes, on the module's own - ``DeliveryWorker``, so they never delay those (see ``workers``). +- dataset bundle uploads and release changes, which can take minutes, each + on the module's own ``DeliveryWorker``, so they never delay those (see + ``workers``). """ from __future__ import annotations @@ -21,6 +22,7 @@ from microcosm_emitter.huggingface.service.delivery import ( CREDENTIAL_MISSING, DATASET_BUNDLE, + RELEASE_CHANGE, HuggingFaceDelivery, ) from microcosm_emitter.huggingface.service.hub import ( @@ -28,8 +30,13 @@ hugging_face_api, ) from microcosm_emitter.huggingface.service.queue import HandoverQueue, sha256_bytes +from microcosm_emitter.huggingface.service.releases import ( + ReleaseChangePolicy, + allow_every_change, +) from microcosm_emitter.huggingface.service.requests import ( RequestRefusedError, + parse_release_change, parse_stage_dataset_bundle, parse_stage_run_files, require_supported_version, @@ -79,11 +86,13 @@ def __init__( *, api_factory: Callable[[], Any | None] = hugging_face_api, clock: Callable[[], float] = time.time, + release_change_policy: ReleaseChangePolicy = allow_every_change, background_workers: bool = True, worker_interval_seconds: float = TICK_SECONDS, ) -> None: self.configuration, self.context = configuration, context self.api_factory, self.clock = api_factory, clock + self.release_change_policy = release_change_policy self.background_workers = background_workers self.worker_interval_seconds = worker_interval_seconds self.queue: HandoverQueue | None = None @@ -96,7 +105,10 @@ def initialize(self) -> None: # ``HandoverQueue``); ``HuggingFaceSettings`` names it. self.queue = HandoverQueue(Path(self.configuration["handover_database_path"])) self.delivery = HuggingFaceDelivery( - self.queue, api_factory=self.api_factory, clock=self.clock + self.queue, + api_factory=self.api_factory, + clock=self.clock, + release_change_policy=self.release_change_policy, ) if self.background_workers: self.workers = self._delivery_workers(self.delivery) @@ -115,7 +127,17 @@ def _delivery_workers( ), interval_seconds=self.worker_interval_seconds, on_error=self._worker_error, - ) + ), + # A publication uploads release files too; it never waits for a + # bundle upload to another repository, nor holds one up. + RELEASE_CHANGE: DeliveryWorker( + "release-changes", + lambda stop_requested: delivery.job_cycle( + frozenset({RELEASE_CHANGE}), stop_requested=stop_requested + ), + interval_seconds=self.worker_interval_seconds, + on_error=self._worker_error, + ), } def _worker_error(self) -> None: @@ -149,6 +171,8 @@ def query(self, message: JsonObject) -> dict[str, Any]: return {"receipt": self._handover_receipt(message)} if command == "check_write_access": return {"access": self._check_write_access(message)} + if command in {"publish_release", "promote_build", "withdraw_build"}: + return {"receipt": self._release_change(message)} raise ValueError("unsupported Hugging Face hand-over command") def _stage_dataset_bundle(self, message: JsonObject) -> dict[str, Any]: @@ -186,6 +210,41 @@ def _stage_dataset_bundle(self, message: JsonObject) -> dict[str, Any]: files=hand_over.copied, ) + def _release_change(self, message: JsonObject) -> dict[str, Any]: + assert self.queue is not None + try: + hand_over = parse_release_change(message) + except RequestRefusedError as refusal: + return { + "handover_id": "", + "status": "refused", + "error_code": refusal.code, + "message": None, + "commit": None, + "repository": str(message.get("repository") or ""), + "release": str(message.get("release") or ""), + "build": str(message.get("build") or ""), + "attempts": 0, + } + return self.queue.enqueue_job( + handover_id=hand_over.handover_id, + kind=RELEASE_CHANGE, + target=hand_over.repository, + request=hand_over.request, + receipt={ + "handover_id": hand_over.handover_id, + "status": "pending", + "error_code": None, + "message": None, + "commit": None, + "repository": hand_over.repository, + "release": hand_over.release, + "build": hand_over.build, + "attempts": 0, + }, + now=self.clock(), + ) + def _handover_receipt(self, message: JsonObject) -> dict[str, Any] | None: assert self.queue is not None require_supported_version(message) diff --git a/src/microcosm_emitter/huggingface/service/queue.py b/src/microcosm_emitter/huggingface/service/queue.py index edd883d9..eb9fd49f 100644 --- a/src/microcosm_emitter/huggingface/service/queue.py +++ b/src/microcosm_emitter/huggingface/service/queue.py @@ -239,14 +239,11 @@ def due_repositories( due = [] for repository in sorted(run_file_repositories | job_repositories): schedule = session.get(RepositoryCommitSchedule, repository) - if schedule is not None and ( - schedule.lease_until > now - or schedule.last_commit_at + MIN_COMMIT_SPACING_SECONDS > now - ): + if schedule is not None and schedule.lease_until > now: continue - run_files_allowed = schedule is None or schedule.retry_after <= now if repository in job_repositories or ( - repository in run_file_repositories and run_files_allowed + repository in run_file_repositories + and self._run_files_allowed(schedule, now) ): due.append(repository) return due diff --git a/src/microcosm_emitter/huggingface/service/releases.py b/src/microcosm_emitter/huggingface/service/releases.py new file mode 100644 index 00000000..6fb5c64a --- /dev/null +++ b/src/microcosm_emitter/huggingface/service/releases.py @@ -0,0 +1,255 @@ +"""The release writer: the only code that changes a dataset repository's +``releases.json``. + +Each change (publish a build, make a build current, withdraw a build) is one +commit: + +1. read ``releases.json`` and the file list at ``main``'s head; +2. apply the change to the record; +3. run the shared checks in ``microcosm_emitter_interface.releases``; +4. commit the new build folder (for a publication) and ``releases.json`` + with ``parent_commit`` set to that head. + +If another commit landed first, the Hub refuses the stale parent; the writer +then reads the new head and applies the change again, so concurrent +publications never lose each other's changes. The writer creates no Hugging +Face tags or branches, no ``current/`` folder and no ``latest*.json`` file, +and never touches existing tags or files outside the new build folder. + +Who may make a change is not settled; ``ReleaseChangePolicy`` is where that +decision plugs in, and the default allows every change. +""" + +from __future__ import annotations + +import hashlib +from collections.abc import Callable, Mapping +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +from microcosm_emitter.huggingface.service.hub import ( + DatasetRepository, + add_operation, + is_parent_conflict, +) +from microcosm_emitter_interface.releases import ( + RELEASE_MANIFEST_NAME, + RELEASES_RECORD_PATH, + ReleaseRecordError, + ReleasesRecord, + add_build, + build_folder, + check_change, + check_commit, + check_repository, + make_current, + withdraw_build, +) + +MAX_PARENT_CONFLICT_ATTEMPTS = 5 + +#: Returns ``None`` to allow a change, or a reason to refuse it. It receives +#: the change's request (``command``, ``repository``, ``release``, ``build``, +#: ...). Not settled yet: who may promote or withdraw. +type ReleaseChangePolicy = Callable[[Mapping[str, Any]], str | None] + + +def allow_every_change(change: Mapping[str, Any]) -> str | None: + return None + + +@dataclass(frozen=True) +class ReleaseOutcome: + """One attempt's result: final unless ``retry``.""" + + status: str + error_code: str | None = None + message: str | None = None + commit: str | None = None + retry: bool = False + committed: bool = False + + +def _refused(code: str, message: str) -> ReleaseOutcome: + return ReleaseOutcome("refused", code, message) + + +def _pending(code: str) -> ReleaseOutcome: + return ReleaseOutcome("pending", code, retry=True) + + +def _sha256_file(path: Path) -> str: + digest = hashlib.sha256() + with path.open("rb") as stream: + for block in iter(lambda: stream.read(1024 * 1024), b""): + digest.update(block) + return digest.hexdigest() + + +def _preserved_files(request: Mapping[str, Any]) -> ReleaseOutcome | None: + for entry in request.get("files", []): + path = Path(entry["local_path"]) + try: + if not path.is_file(): + return _pending("PRESERVED_FILES_MISSING") + if ( + path.stat().st_size != entry["bytes"] + or _sha256_file(path) != entry["sha256"] + ): + return _refused( + "PRESERVED_BYTES_CHANGED", + f"{entry['name']} changed after it was handed over.", + ) + except OSError: + return _pending("PRESERVED_FILES_MISSING") + return None + + +def _commit_message(request: Mapping[str, Any]) -> str: + release, build = request["release"], request["build"] + command = request["command"] + if command == "publish_release": + suffix = " and make it current" if request["make_current"] else "" + return f"Publish build {build} of {release}{suffix}" + if command == "promote_build": + return f"Make build {build} the current build of {release}" + return f"Withdraw build {build} of {release}" + + +def _first_violation(error: ReleaseRecordError) -> ReleaseOutcome: + first = error.violations[0] + return _refused(first.code, "; ".join(item.message for item in error.violations)) + + +def apply_release_change( + repository: DatasetRepository, + request: Mapping[str, Any], + *, + published_at: str, + policy: ReleaseChangePolicy = allow_every_change, +) -> ReleaseOutcome: + """Apply one release change, or report why it did not land.""" + refusal = policy(request) + if refusal is not None: + return _refused("NOT_AUTHORIZED", refusal) + publishing = request["command"] == "publish_release" + if publishing: + preserved = _preserved_files(request) + if preserved is not None: + return preserved + release, build = request["release"], request["build"] + folder = build_folder(release, build) + new_files = { + f"{folder}/{entry['name']}": Path(entry["local_path"]) + for entry in (request["files"] if publishing else []) + } + for _ in range(MAX_PARENT_CONFLICT_ATTEMPTS): + try: + head = repository.head_revision() + files = repository.list_files(revision=head) + before = ( + ReleasesRecord.parse( + repository.download(RELEASES_RECORD_PATH, revision=head) + ) + if RELEASES_RECORD_PATH in files + else None + ) + except ReleaseRecordError as error: + return _first_violation(error) + except Exception: + return _pending("REPOSITORY_UNAVAILABLE") + record = before or ReleasesRecord.empty() + try: + if publishing: + if any(path.startswith(folder + "/") for path in files): + return _already_published(repository, request, record, head) + after = add_build( + record, + release=release, + build=build, + release_role=request["release_role"], + published_at=published_at, + make_current=request["make_current"], + ) + elif request["command"] == "promote_build": + after = make_current(record, release=release, build=build) + else: + after = withdraw_build(record, release=release, build=build) + except ReleaseRecordError as error: + return _first_violation(error) + added_paths = [*new_files, RELEASES_RECORD_PATH] + + def read_file(path: str, revision: str = head) -> bytes: + local = new_files.get(path) + if local is not None: + return local.read_bytes() + return repository.download(path, revision=revision) + + try: + violations = [ + *check_change(before, after), + *check_commit( + before_files=files, + added_paths=added_paths, + deleted_paths=[], + before=before, + after=after, + ), + *check_repository(after, [*files, *added_paths], read_file), + ] + except Exception: + return _pending("REPOSITORY_UNAVAILABLE") + if violations: + return _refused( + violations[0].code, "; ".join(item.message for item in violations) + ) + operations = [add_operation(path, local) for path, local in new_files.items()] + operations.append(add_operation(RELEASES_RECORD_PATH, after.serialize())) + try: + commit = repository.commit( + operations, message=_commit_message(request), parent_commit=head + ) + except Exception as error: + if is_parent_conflict(error): + continue + return _pending("UPLOAD_FAILED") + return ReleaseOutcome("published", commit=commit, committed=True) + return _pending("PARENT_CONFLICT") + + +def _already_published( + repository: DatasetRepository, + request: Mapping[str, Any], + record: ReleasesRecord, + head: str, +) -> ReleaseOutcome: + """A build folder that exists is refused, unless this request made it. + + A retried publication whose earlier attempt landed (the receipt was lost) + finds its own build listed with byte-identical manifest: that is + published, not refused. + """ + release, build = request["release"], request["build"] + entry = record.releases.get(release) + manifest_path = f"{build_folder(release, build)}/{RELEASE_MANIFEST_NAME}" + local_manifest = next( + Path(item["local_path"]) + for item in request["files"] + if item["name"] == RELEASE_MANIFEST_NAME + ) + exists_message = f"{build_folder(release, build)}/ already exists." + if entry is None or build not in entry.builds: + return _refused("BUILD_FOLDER_EXISTS", exists_message) + if request["make_current"] and entry.current != build: + return _refused("BUILD_FOLDER_EXISTS", exists_message) + try: + same = repository.download(manifest_path, revision=head) == ( + local_manifest.read_bytes() + ) + commit = repository.last_commit(manifest_path) if same else None + except Exception: + return _pending("REPOSITORY_UNAVAILABLE") + if not same: + return _refused("BUILD_FOLDER_EXISTS", exists_message) + return ReleaseOutcome("published", commit=commit or head) diff --git a/src/microcosm_emitter/huggingface/service/requests.py b/src/microcosm_emitter/huggingface/service/requests.py index 8799ccfb..12f637a5 100644 --- a/src/microcosm_emitter/huggingface/service/requests.py +++ b/src/microcosm_emitter/huggingface/service/requests.py @@ -25,6 +25,13 @@ repository_prefix, safe_identifier, ) +from microcosm_emitter_interface.publication import ( + RELEASE_CHANGE, + PromoteBuild, + PublishRelease, + ReleaseFile, + WithdrawBuild, +) #: Transport bounds for one staging run file and one hand-over. Microcosm's #: staging contract keeps its own, smaller limits. @@ -240,3 +247,74 @@ def parse_stage_dataset_bundle(message: Mapping[str, Any]) -> BundleHandOver: request={key: value for key, value in canonical.items()}, copied=copied, ) + + +@dataclass(frozen=True) +class ReleaseHandOver: + handover_id: str + repository: str + release: str + build: str + request: dict[str, Any] + + +def parse_release_change(message: Mapping[str, Any]) -> ReleaseHandOver: + """Check a publication, promotion or withdrawal with the interface's rules.""" + require_supported_version(message) + command = message.get("command") + try: + if command == "publish_release": + entries = message.get("files") + if not isinstance(entries, list) or any( + not isinstance(entry, Mapping) for entry in entries + ): + raise RequestRefusedError("INVALID_REQUEST") + request = PublishRelease( + repository=message.get("repository"), + release=message.get("release"), + build=message.get("build"), + release_role=message.get("release_role"), + make_current=message.get("make_current"), + files=tuple( + ReleaseFile( + name=entry.get("name"), + role=entry.get("role"), + local_path=Path(str(entry.get("local_path"))), + sha256=entry.get("sha256"), + bytes=entry.get("bytes"), + ) + for entry in entries + ), + ) + elif command in {"promote_build", "withdraw_build"}: + change = PromoteBuild if command == "promote_build" else WithdrawBuild + request = change( + repository=message.get("repository"), + release=message.get("release"), + build=message.get("build"), + ) + else: + raise RequestRefusedError("INVALID_REQUEST") + except (ValueError, TypeError) as error: + if isinstance(error, RequestRefusedError): + raise + raise RequestRefusedError("INVALID_REQUEST") from None + if command == "publish_release": + for entry, item in zip(message["files"], request.files, strict=True): + path = Path(str(entry.get("local_path"))) + if not path.is_absolute(): + raise RequestRefusedError("INVALID_REQUEST") + try: + if not path.is_file(): + raise RequestRefusedError("LOCAL_FILE_MISSING") + if path.stat().st_size != item.bytes: + raise RequestRefusedError("FILE_SIZE_DIFFERS") + except OSError: + raise RequestRefusedError("LOCAL_FILE_MISSING") from None + return ReleaseHandOver( + handover_id=handover_identifier(RELEASE_CHANGE, dict(message)), + repository=request.repository, + release=request.release, + build=request.build, + request=dict(message), + ) diff --git a/tests/huggingface/test_release_writer.py b/tests/huggingface/test_release_writer.py new file mode 100644 index 00000000..ad09d715 --- /dev/null +++ b/tests/huggingface/test_release_writer.py @@ -0,0 +1,405 @@ +"""The release writer: build folders side by side and one releases.json.""" + +import hashlib +import json +import os + +import pytest +from fake_hugging_face import PRIVATE_REPOSITORY, FakeHubHTTPError + +from microcosm_emitter.host.contracts import ModuleContext +from microcosm_emitter.huggingface.service.hub import DatasetRepository +from microcosm_emitter.huggingface.service.module import HuggingFaceModule +from microcosm_emitter.huggingface.service.releases import ( + allow_every_change, + apply_release_change, +) +from microcosm_emitter_interface.publication import ( + PromoteBuild, + PublishRelease, + ReleaseFile, + WithdrawBuild, +) +from microcosm_emitter_interface.releases import ReleasesRecord + +NATIONAL = "microcosm-uk-2024-25-national" +FIRST_BUILD = "uk-frs-calibration-attempt-20261014T214443Z-d12ff9b3" +SECOND_BUILD = "uk-frs-calibration-attempt-20261019T083323Z-6cba5d71" + + +def release_files(directory, release, build, *, manifest_build=None, dataset=b"rows"): + directory.mkdir(parents=True, exist_ok=True) + contents = { + "release_manifest.json": ( + "release_manifest", + json.dumps( + {"release": release, "build": manifest_build or build}, indent=2 + ).encode(), + ), + "microcosm_uk_2024_25.h5": ("dataset", dataset), + "calibration_diagnostics.json": ("calibration_diagnostics", b"{}"), + } + files = [] + for name, (role, content) in contents.items(): + (directory / name).write_bytes(content) + files.append( + ReleaseFile( + name=name, + role=role, + local_path=directory / name, + sha256=hashlib.sha256(content).hexdigest(), + bytes=len(content), + ) + ) + return tuple(files), {name: content for name, (_, content) in contents.items()} + + +def publish_request(tmp_path, build, *, make_current=False, **options): + files, contents = release_files(tmp_path / build, NATIONAL, build, **options) + return ( + PublishRelease( + repository=PRIVATE_REPOSITORY, + release=NATIONAL, + build=build, + release_role="national_default", + make_current=make_current, + files=files, + ), + contents, + ) + + +def apply(hub, request, *, published_at="2026-10-20T00:00:00+00:00", policy=None): + return apply_release_change( + DatasetRepository(PRIVATE_REPOSITORY, hub), + request.message(), + published_at=published_at, + policy=policy or allow_every_change, + ) + + +def record_of(hub) -> ReleasesRecord: + return ReleasesRecord.parse(hub.files(PRIVATE_REPOSITORY)["releases.json"]) + + +def no_tags_or_branches(hub) -> bool: + return ( + not any(method in {"create_tag", "create_branch"} for method, _ in hub.calls) + and not hub.repositories[PRIVATE_REPOSITORY].tags + ) + + +def test_publication_writes_the_build_folder_and_releases_json_in_one_commit( + hub, tmp_path +): + head_before = hub.repositories[PRIVATE_REPOSITORY].head.oid + request, contents = publish_request(tmp_path, FIRST_BUILD) + outcome = apply(hub, request) + assert outcome.status == "published" + (commit,) = hub.commits(PRIVATE_REPOSITORY) + assert outcome.commit == commit.oid and commit.parent == head_before + folder = f"releases/{NATIONAL}/{FIRST_BUILD}" + assert commit.added == { + **{f"{folder}/{name}": content for name, content in contents.items()}, + "releases.json": commit.added["releases.json"], + } + assert json.loads(commit.added["releases.json"]) == { + "schema_version": 1, + "default": None, + "releases": { + NATIONAL: { + "role": "national_default", + "current": None, + "builds": { + FIRST_BUILD: { + "published_at": "2026-10-20T00:00:00+00:00", + "withdrawn": False, + } + }, + } + }, + } + assert commit.message == f"Publish build {FIRST_BUILD} of {NATIONAL}" + (create_commit,) = [ + arguments for method, arguments in hub.calls if method == "create_commit" + ] + assert create_commit["parent_commit"] == head_before + assert no_tags_or_branches(hub) + + +def test_a_published_build_can_become_current_in_the_same_commit(hub, tmp_path): + request, _ = publish_request(tmp_path, FIRST_BUILD, make_current=True) + assert apply(hub, request).status == "published" + assert record_of(hub).releases[NATIONAL].current == FIRST_BUILD + (commit,) = hub.commits(PRIVATE_REPOSITORY) + assert commit.message.endswith("and make it current") + + +def test_publication_refuses_a_build_whose_folder_exists(hub, tmp_path): + hub.write_directly( + PRIVATE_REPOSITORY, {f"releases/{NATIONAL}/{FIRST_BUILD}/old.json": b"{}"} + ) + request, _ = publish_request(tmp_path, FIRST_BUILD) + outcome = apply(hub, request) + assert (outcome.status, outcome.error_code) == ("refused", "BUILD_FOLDER_EXISTS") + assert [commit.message for commit in hub.commits(PRIVATE_REPOSITORY)] == ["Direct"] + + +def test_repeating_a_publication_that_already_landed_reports_it(hub, tmp_path): + request, _ = publish_request(tmp_path, FIRST_BUILD) + first = apply(hub, request) + again = apply(hub, request, published_at="2026-10-21T00:00:00+00:00") + assert (again.status, again.commit) == ("published", first.commit) + assert len(hub.commits(PRIVATE_REPOSITORY)) == 1 + + +def test_concurrent_publications_of_one_release_both_land(hub, tmp_path): + other, _ = publish_request(tmp_path, FIRST_BUILD) + ours, _ = publish_request(tmp_path, SECOND_BUILD, make_current=True) + hub.run_before_next_commit(lambda: apply(hub, other)) + outcome = apply(hub, ours) + assert outcome.status == "published" + record = record_of(hub) + assert set(record.releases[NATIONAL].builds) == {FIRST_BUILD, SECOND_BUILD} + assert record.releases[NATIONAL].current == SECOND_BUILD + our_attempts = [ + arguments + for method, arguments in hub.calls + if method == "create_commit" + and arguments["commit_message"].startswith(f"Publish build {SECOND_BUILD}") + ] + assert len(our_attempts) == 2 # one refused on its stale parent, one landed + assert len(hub.commits(PRIVATE_REPOSITORY)) == 2 + + +def test_current_cannot_change_while_pointer_files_exist(hub, tmp_path): + hub.write_directly(PRIVATE_REPOSITORY, {"latest-national.json": b"{}"}) + promoting, _ = publish_request(tmp_path, FIRST_BUILD, make_current=True) + refused = apply(hub, promoting) + assert (refused.status, refused.error_code) == ("refused", "POINTER_FILES_PRESENT") + listing, _ = publish_request(tmp_path / "again", FIRST_BUILD) + assert apply(hub, listing).status == "published" + promotion = apply( + hub, + PromoteBuild( + repository=PRIVATE_REPOSITORY, release=NATIONAL, build=FIRST_BUILD + ), + ) + assert (promotion.status, promotion.error_code) == ( + "refused", + "POINTER_FILES_PRESENT", + ) + assert hub.files(PRIVATE_REPOSITORY)["latest-national.json"] == b"{}" + + +def test_promotion_and_withdrawal_change_only_releases_json(hub, tmp_path): + for build in (FIRST_BUILD, SECOND_BUILD): + request, _ = publish_request(tmp_path, build, make_current=True) + assert apply(hub, request).status == "published" + promoted = apply( + hub, + PromoteBuild( + repository=PRIVATE_REPOSITORY, release=NATIONAL, build=FIRST_BUILD + ), + ) + assert promoted.status == "published" + assert list(hub.commits(PRIVATE_REPOSITORY)[-1].added) == ["releases.json"] + withdrawn = apply( + hub, + WithdrawBuild( + repository=PRIVATE_REPOSITORY, release=NATIONAL, build=SECOND_BUILD + ), + ) + assert withdrawn.status == "published" + assert list(hub.commits(PRIVATE_REPOSITORY)[-1].added) == ["releases.json"] + record = record_of(hub) + assert record.releases[NATIONAL].current == FIRST_BUILD + assert record.releases[NATIONAL].builds[SECOND_BUILD].withdrawn is True + assert f"releases/{NATIONAL}/{SECOND_BUILD}/release_manifest.json" in hub.files( + PRIVATE_REPOSITORY + ) + current = apply( + hub, + WithdrawBuild( + repository=PRIVATE_REPOSITORY, release=NATIONAL, build=FIRST_BUILD + ), + ) + assert (current.status, current.error_code) == ( + "refused", + "WITHDRAWING_CURRENT_BUILD", + ) + assert no_tags_or_branches(hub) + + +def test_a_manifest_naming_another_build_is_refused(hub, tmp_path): + request, _ = publish_request(tmp_path, FIRST_BUILD, manifest_build=SECOND_BUILD) + outcome = apply(hub, request) + assert (outcome.status, outcome.error_code) == ("refused", "MANIFEST_MISMATCH") + assert hub.commits(PRIVATE_REPOSITORY) == [] + + +def test_an_invalid_record_in_the_repository_is_refused_until_repaired(hub, tmp_path): + hub.write_directly(PRIVATE_REPOSITORY, {"releases.json": b'{"schema_version": 2}'}) + request, _ = publish_request(tmp_path, FIRST_BUILD) + outcome = apply(hub, request) + assert (outcome.status, outcome.error_code) == ("refused", "INVALID_RECORD") + + +def test_a_listed_build_without_its_folder_blocks_every_write(hub, tmp_path): + listed_only = { + "schema_version": 1, + "default": None, + "releases": { + NATIONAL: { + "role": "national_default", + "current": None, + "builds": { + "lost-build": {"published_at": "2026-10-01", "withdrawn": False} + }, + } + }, + } + hub.write_directly( + PRIVATE_REPOSITORY, {"releases.json": json.dumps(listed_only).encode()} + ) + request, _ = publish_request(tmp_path, FIRST_BUILD) + outcome = apply(hub, request) + assert (outcome.status, outcome.error_code) == ("refused", "BUILD_FOLDER_MISSING") + + +@pytest.mark.parametrize( + ("method", "error_code"), + [("repo_info", "REPOSITORY_UNAVAILABLE"), ("create_commit", "UPLOAD_FAILED")], +) +def test_hub_failures_leave_the_change_pending_for_a_retry( + hub, tmp_path, method, error_code +): + hub.fail_next(method, FakeHubHTTPError(503)) + request, _ = publish_request(tmp_path, FIRST_BUILD) + outcome = apply(hub, request) + assert (outcome.status, outcome.error_code, outcome.retry) == ( + "pending", + error_code, + True, + ) + assert apply(hub, request).status == "published" + + +def test_local_bytes_changed_after_the_hand_over_are_refused(hub, tmp_path): + request, _ = publish_request(tmp_path, FIRST_BUILD) + dataset = next(item for item in request.files if item.role == "dataset") + dataset.local_path.write_bytes(b"X" * dataset.bytes) + outcome = apply(hub, request) + assert (outcome.status, outcome.error_code) == ( + "refused", + "PRESERVED_BYTES_CHANGED", + ) + assert hub.commits(PRIVATE_REPOSITORY) == [] + + +def test_a_change_policy_can_refuse_a_promotion(hub, tmp_path): + request, _ = publish_request(tmp_path, FIRST_BUILD) + assert apply(hub, request).status == "published" + + def only_listing(change): + return None if change["command"] == "publish_release" else "not permitted" + + outcome = apply( + hub, + PromoteBuild( + repository=PRIVATE_REPOSITORY, release=NATIONAL, build=FIRST_BUILD + ), + policy=only_listing, + ) + assert (outcome.status, outcome.error_code) == ("refused", "NOT_AUTHORIZED") + + +def test_existing_tags_old_files_and_staged_bundles_are_untouched(hub, tmp_path): + old_files = { + f"releases/{NATIONAL}/release_manifest.json": b'{"old": true}', + "staged/uk-run/staged_manifest.json": b"{}", + "latest.json": b"{}", + } + tagged = hub.write_directly(PRIVATE_REPOSITORY, old_files) + hub.repositories[PRIVATE_REPOSITORY].tags[ + "microcosm-uk-2024-25-national-20261014T214443Z-d12ff9b3" + ] = tagged + request, _ = publish_request(tmp_path, FIRST_BUILD) + assert apply(hub, request).status == "published" + files = hub.files(PRIVATE_REPOSITORY) + for path, content in old_files.items(): + assert files[path] == content + assert hub.repositories[PRIVATE_REPOSITORY].tags == { + "microcosm-uk-2024-25-national-20261014T214443Z-d12ff9b3": tagged + } + assert not any( + path.rsplit("/", 1)[-1].startswith("latest") + for path in hub.commits(PRIVATE_REPOSITORY)[-1].added + ) + + +def test_module_queues_a_release_change_and_reports_its_receipt( + hugging_face_module, +): + controls = hugging_face_module + request, _ = publish_request(controls.tmp_path, FIRST_BUILD, make_current=True) + receipt = controls.module.query(request.message())["receipt"] + assert (receipt["status"], receipt["handover_id"]) == ( + "pending", + request.handover_id(), + ) + controls.module.delivery.run_cycle() + receipt = controls.module.query( + { + "protocol_version": 1, + "command": "handover_receipt", + "handover_id": request.handover_id(), + } + )["receipt"] + assert receipt["status"] == "published" + assert receipt["commit"] == controls.hub.commits(PRIVATE_REPOSITORY)[-1].oid + assert (receipt["release"], receipt["build"]) == (NATIONAL, FIRST_BUILD) + record = ReleasesRecord.parse( + controls.hub.files(PRIVATE_REPOSITORY)["releases.json"] + ) + assert record.releases[NATIONAL].current == FIRST_BUILD + + +def test_module_refuses_a_malformed_release_change_without_queueing_it( + hugging_face_module, +): + controls = hugging_face_module + request, _ = publish_request(controls.tmp_path, FIRST_BUILD) + message = request.message() + message["files"] = [ + entry for entry in message["files"] if entry["role"] != "release_manifest" + ] + receipt = controls.module.query(message)["receipt"] + assert (receipt["status"], receipt["error_code"]) == ("refused", "INVALID_REQUEST") + controls.module.delivery.run_cycle() + assert controls.hub.commits(PRIVATE_REPOSITORY) == [] + + +def test_the_module_writer_uses_the_injected_policy(tmp_path, hub): + module = HuggingFaceModule( + {"handover_database_path": str(tmp_path / "events.handovers.sqlite3")}, + ModuleContext(os.getpid()), + api_factory=lambda: hub, + release_change_policy=lambda change: "refused by policy", + background_workers=False, + ) + module.initialize() + try: + request, _ = publish_request(tmp_path, FIRST_BUILD) + module.query(request.message()) + module.delivery.run_cycle() + receipt = module.query( + { + "protocol_version": 1, + "command": "handover_receipt", + "handover_id": request.handover_id(), + } + )["receipt"] + finally: + module.close(0) + assert (receipt["status"], receipt["error_code"]) == ("refused", "NOT_AUTHORIZED") diff --git a/tests/interface/test_client_messages.py b/tests/interface/test_client_messages.py index 37c1c0e3..5a264429 100644 --- a/tests/interface/test_client_messages.py +++ b/tests/interface/test_client_messages.py @@ -15,6 +15,12 @@ materialize, protocol_fixtures, ) +from microcosm_emitter_interface.publication import ( + PromoteBuild, + PublishRelease, + ReleaseFile, + WithdrawBuild, +) from microcosm_emitter_interface.staging import ( HuggingFaceSettings, RunFile, @@ -26,6 +32,29 @@ FIXTURE_PRIVATE_REPOSITORY = "policyengine/fixture-private" +def fixture_publication(directory: Path) -> PublishRelease: + """The publication recorded in huggingface-publish-release.json.""" + fixture = fixtures_by_name()["huggingface-publish-release"] + recorded = materialize(fixture, directory=directory, spool_path=Path("unused")) + return PublishRelease( + repository=FIXTURE_PRIVATE_REPOSITORY, + release="fixture-release", + build="fixture-build", + release_role="national_default", + make_current=True, + files=tuple( + ReleaseFile( + name=entry["name"], + role=entry["role"], + local_path=Path(entry["local_path"]), + sha256=entry["sha256"], + bytes=entry["bytes"], + ) + for entry in recorded["files"] + ), + ) + + def fixture_bundle(directory: Path) -> StageDatasetBundle: """The bundle recorded in huggingface-stage-dataset-bundle.json.""" fixture = fixtures_by_name()["huggingface-stage-dataset-bundle"] @@ -218,6 +247,25 @@ def fixtures_by_name() -> dict[str, ProtocolFixture]: "huggingface-check-write-access": lambda client, directory: ( client.check_write_access(FIXTURE_PRIVATE_REPOSITORY) ), + "huggingface-publish-release": lambda client, directory: client.publish_release( + fixture_publication(directory), wait_seconds=0 + ), + "huggingface-promote-build": lambda client, directory: client.promote_build( + PromoteBuild( + repository=FIXTURE_PRIVATE_REPOSITORY, + release="fixture-release", + build="fixture-build", + ), + wait_seconds=0, + ), + "huggingface-withdraw-build": lambda client, directory: client.withdraw_build( + WithdrawBuild( + repository=FIXTURE_PRIVATE_REPOSITORY, + release="fixture-release", + build="fixture-build", + ), + wait_seconds=0, + ), "huggingface-read-back-run-files": lambda client, directory: ( client.read_back_run_files( FIXTURE_STAGING_REPOSITORY, "fixture-run", wait_seconds=0 diff --git a/tests/interface/test_protocol_fixtures.py b/tests/interface/test_protocol_fixtures.py index 10aff546..df87ed7d 100644 --- a/tests/interface/test_protocol_fixtures.py +++ b/tests/interface/test_protocol_fixtures.py @@ -37,6 +37,15 @@ "protocol-v1/host-close.json": ( "68fa81b4116852f91f3c0dab3716a1974a8150039fb2b238f79efc094d9b0f6f" ), + "protocol-v1/huggingface-promote-build.json": ( + "ad6bddbb09742bf45bf2d64c6c605c7641d28194e74b376c42a7872318d5b2bf" + ), + "protocol-v1/huggingface-publish-release.json": ( + "c7733eae51dd38f140cd2949583d86119adef38ff943db5b0e5ef1038441dc61" + ), + "protocol-v1/huggingface-withdraw-build.json": ( + "b02ff56e8697ec7cc6e7658699794bb80d8f61504d970f78f9fdac19b823893c" + ), "protocol-v1/huggingface-check-write-access.json": ( "750b7eb8c7c9ea4112bb60560522d3b1591c6c9838239a5a54d72f90c095a631" ), diff --git a/tests/interface/test_release_client.py b/tests/interface/test_release_client.py new file mode 100644 index 00000000..e0b8007d --- /dev/null +++ b/tests/interface/test_release_client.py @@ -0,0 +1,177 @@ +"""Typed release publication: release, build, whether current, files with roles.""" + +import hashlib +import json +from pathlib import Path + +import pytest +from fake_hugging_face import PRIVATE_REPOSITORY + +from microcosm_emitter_interface.client import EmitterClient +from microcosm_emitter_interface.publication import ( + PromoteBuild, + PublishRelease, + ReleaseFile, + WithdrawBuild, +) +from microcosm_emitter_interface.releases import ReleasesRecord +from microcosm_emitter_interface.transport import SocketTransport + +RELEASE = "microcosm-uk-2024-25-national" +BUILD = "uk-frs-calibration-attempt-20261019T083323Z-6cba5d71" + + +def release_file(directory: Path, name: str, role: str, content: bytes) -> ReleaseFile: + directory.mkdir(parents=True, exist_ok=True) + (directory / name).write_bytes(content) + return ReleaseFile( + name=name, + role=role, + local_path=directory / name, + sha256=hashlib.sha256(content).hexdigest(), + bytes=len(content), + ) + + +def build_files(directory: Path, build: str = BUILD) -> tuple[ReleaseFile, ...]: + return ( + release_file( + directory, + "release_manifest.json", + "release_manifest", + json.dumps({"release": RELEASE, "build": build}).encode(), + ), + release_file(directory, "microcosm_uk_2024_25.h5", "dataset", b"rows"), + ) + + +def publication(directory: Path, **changes) -> PublishRelease: + arguments = { + "repository": PRIVATE_REPOSITORY, + "release": RELEASE, + "build": BUILD, + "release_role": "national_default", + "make_current": False, + } + arguments.update(changes) + if "files" not in arguments: + arguments["files"] = build_files(directory) + return PublishRelease(**arguments) + + +@pytest.mark.parametrize( + "changes", + [ + lambda files: {"files": files[1:]}, + lambda files: { + "files": ( + ReleaseFile( + name="release_manifest.json", + role="dataset", + local_path=files[0].local_path, + sha256=files[0].sha256, + bytes=files[0].bytes, + ), + files[1], + ) + }, + lambda files: { + "files": files + + ( + ReleaseFile( + name="latest-national.json", + role="pointer", + local_path=files[0].local_path, + sha256=files[0].sha256, + bytes=files[0].bytes, + ), + ) + }, + lambda files: {"files": files + (files[1],)}, + lambda files: {"release": "../escape"}, + lambda files: {"build": "two/parts"}, + lambda files: {"release_role": "National Default"}, + lambda files: {"make_current": "yes"}, + ], + ids=[ + "no-manifest", + "manifest-role", + "pointer-file", + "duplicate-file", + "release", + "build", + "role", + "make-current", + ], +) +def test_publications_that_break_the_rules_cannot_be_built(tmp_path, changes): + with pytest.raises(ValueError): + publication(tmp_path, **changes(build_files(tmp_path))) + + +def test_publication_message_names_release_build_and_each_file_role(tmp_path): + request = publication(tmp_path, make_current=True) + message = request.message() + assert {key: message[key] for key in ("command", "release", "build")} == { + "command": "publish_release", + "release": RELEASE, + "build": BUILD, + } + assert message["make_current"] is True + assert message["release_role"] == "national_default" + assert [(entry["name"], entry["role"]) for entry in message["files"]] == [ + ("release_manifest.json", "release_manifest"), + ("microcosm_uk_2024_25.h5", "dataset"), + ] + assert ( + request.handover_id() == publication(tmp_path, make_current=True).handover_id() + ) + assert request.handover_id() != publication(tmp_path).handover_id() + + +def test_client_publishes_promotes_and_withdraws_through_the_emitter( + tmp_path, emitter_with_hub +): + client, hub = emitter_with_hub + first = publication(tmp_path / "first", make_current=True) + receipt = client.publish_release(first, wait_seconds=20) + assert receipt["status"] == "published", receipt + second_build = "uk-frs-calibration-attempt-20261020T000000Z-0000aaaa" + second = publication( + tmp_path / "second", + build=second_build, + files=build_files(tmp_path / "second", second_build), + ) + second_receipt = client.publish_release(second, wait_seconds=20) + assert second_receipt["status"] == "published", second_receipt + promoted = client.promote_build( + PromoteBuild( + repository=PRIVATE_REPOSITORY, release=RELEASE, build=second_build + ), + wait_seconds=20, + ) + assert promoted["status"] == "published", promoted + withdrawn = client.withdraw_build( + WithdrawBuild(repository=PRIVATE_REPOSITORY, release=RELEASE, build=BUILD), + wait_seconds=20, + ) + assert withdrawn["status"] == "published", withdrawn + record = ReleasesRecord.parse(hub.files(PRIVATE_REPOSITORY)["releases.json"]) + assert record.releases[RELEASE].current == second_build + assert record.releases[RELEASE].builds[BUILD].withdrawn is True + assert client.release_receipt(first.handover_id())["status"] == "published" + + +def test_client_without_the_module_reports_unavailable(tmp_path, in_thread_emitter): + client = EmitterClient( + in_thread_emitter( + SocketTransport(tmp_path / "unused.sock"), + {"modules": ["telemetry"], "protocol_versions": [1]}, + ) + ) + receipt = client.publish_release(publication(tmp_path), wait_seconds=1) + assert (receipt["status"], receipt["release"], receipt["build"]) == ( + "unavailable", + RELEASE, + BUILD, + ) From 43128c699486af64bd7c5587961993ab66ec1ab0 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 20 Oct 2026 17:17:49 +0400 Subject: [PATCH 03/24] Build release messages inside the client's error handling `publish_release`, `promote_build` and `withdraw_build` built their message before the client's error handling, and the unavailable receipt rebuilt it, so a release file whose path cannot be resolved raised into the build. The release calls now pass a message builder like every other call, and the unavailable receipt has an empty `handover_id` when the files cannot be converted. Co-Authored-By: Claude Opus 5.5 --- .../src/microcosm_emitter_interface/client.py | 3 +- .../publication.py | 10 +++++- tests/interface/test_client_errors.py | 34 +++++++++++++++++++ 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/client.py b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/client.py index c90161ce..4bbfa9f0 100644 --- a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/client.py +++ b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/client.py @@ -497,10 +497,9 @@ def check_write_access(self, repository: str) -> WriteAccess: def _release_change( self, request: ReleaseChange, wait_seconds: float ) -> ReleaseReceipt: - message = request.message() response = self._query( HUGGING_FACE_MODULE, - lambda: message, + request.message, kind="release", timeout=FILE_REQUEST_TIMEOUT_SECONDS, ) diff --git a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/publication.py b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/publication.py index f7eaf1f6..143c8118 100644 --- a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/publication.py +++ b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/publication.py @@ -208,8 +208,16 @@ class ReleaseReceipt(TypedDict): def unavailable_release_receipt(request: ReleaseChange) -> ReleaseReceipt: + """No emitter accepted the change; built without converting its files. + + ``handover_id`` is empty when the request's files cannot be converted. + """ + try: + handover_id = request.handover_id() + except Exception: + handover_id = "" return { - "handover_id": request.handover_id(), + "handover_id": handover_id, "status": "unavailable", "error_code": None, "message": None, diff --git a/tests/interface/test_client_errors.py b/tests/interface/test_client_errors.py index e8de8cba..1133eeae 100644 --- a/tests/interface/test_client_errors.py +++ b/tests/interface/test_client_errors.py @@ -360,3 +360,37 @@ def test_a_pending_bundle_receipt_without_an_identifier_is_returned_not_raised( receipt = client.stage_dataset_bundle(request, wait_seconds=1) assert receipt == {"status": "pending"} + + +@pytest.mark.parametrize("running", [True, False], ids=["running", "no-emitter"]) +def test_unconvertible_release_arguments_report_failure(running): + import hashlib + + from microcosm_emitter_interface.publication import PublishRelease, ReleaseFile + + client, transport = hugging_face_client() if running else (EmitterClient(), None) + request = PublishRelease( + repository="policyengine/fixture-private", + release="fixture-release", + build="fixture-build", + release_role="national_default", + make_current=True, + files=( + ReleaseFile( + name="release_manifest.json", + role="release_manifest", + local_path=UnresolvablePath(), + sha256=hashlib.sha256(b"").hexdigest(), + bytes=0, + ), + ), + ) + receipt = client.publish_release(request, wait_seconds=0) + assert receipt["status"] == "unavailable" + assert (receipt["release"], receipt["build"]) == ( + "fixture-release", + "fixture-build", + ) + if running: + assert transport.requests == [] + assert client.last_failure.reason == "invalid_arguments" From 0d061a52014b6da13d7091e7f40074dc0cd9fc1c Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 20 Oct 2026 17:20:42 +0400 Subject: [PATCH 04/24] Apply each requested release change; deduplicate only resent requests `PromoteBuild`, `WithdrawBuild` and `PublishRelease` derived their hand-over identifier from the change alone, and `HandoverQueue.enqueue_job` returns the stored receipt for a known identifier. Because the queue database persists across builds, these changes silently did nothing: promoting a build back after another was promoted (A, B, then A again), and retrying a withdrawal after another build was promoted. A refusal such as `POINTER_FILES_PRESENT` or `NOT_AUTHORIZED` also stayed final after its cause was gone. Every release request now carries a `request_id`: a new random one by default, or one the caller gives. The message and the hand-over identifier include it, so a new request is applied even when it repeats an earlier change, and only a resent request (the same `request_id` and change) is deduplicated. The emitter refuses a release change without a valid `request_id` (`INVALID_REQUEST`). The three unreleased release fixtures gain the field. Co-Authored-By: Claude Opus 5.5 --- README.md | 1 + changelog.d/release-writer.added.md | 2 +- .../huggingface-promote-build.json | 1 + .../huggingface-publish-release.json | 1 + .../huggingface-withdraw-build.json | 1 + .../publication.py | 30 +++++- .../huggingface/service/requests.py | 2 + tests/huggingface/test_release_writer.py | 95 +++++++++++++++++++ tests/interface/test_client_messages.py | 3 + tests/interface/test_protocol_fixtures.py | 6 +- tests/interface/test_release_client.py | 54 ++++++++++- tests/support/fake_hugging_face.py | 18 ++++ 12 files changed, 204 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 39baa9ee..29e2301d 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,7 @@ releases///... the build's files, with their rol - Every change is one commit: the writer reads `releases.json` and the file list at `main`'s head, applies the change, runs the shared checks in `microcosm_emitter_interface.releases`, and commits the new build folder and `releases.json` with `parent_commit` set to that head. If another commit lands first, it reads the new head and applies the change again, so concurrent publications keep each other's builds. - The writer refuses a change unless: every listed build has its folder and a `release_manifest.json` naming the same release and build; `current` names a listed build that is not withdrawn; existing build entries change only `withdrawn`; and no file inside an existing build folder is added, changed or removed (so publishing refuses a build whose folder exists). It never writes a `latest*.json` file, and refuses to change `current` while any `latest*.json` file still exists, until the old pointer files are deleted. - It creates no Hugging Face tags or branches, no `current/` folder and no mutable root file, and leaves existing tags and files untouched. It does not port `release.py`'s staging branch, release tag or pointer-file commit, which this layout replaces. +- Each request carries a `request_id`, a new random one unless the caller gives one, and the emitter applies each request once. Sending the same request again returns its receipt; a new request is applied even if it repeats an earlier change (promoting a build back after another was promoted, or retrying a withdrawal or a refused change once its cause is gone). - A retried publication whose earlier attempt already landed reports `published`. Hub failures stay `pending` and are retried; broken rules are `refused` with the rule's code. - `release_change_policy` is where a decision on who may promote or withdraw will plug in; by default every change is allowed. `current` may stay `null`, for a release that is never promoted. diff --git a/changelog.d/release-writer.added.md b/changelog.d/release-writer.added.md index 9f3e464e..675b128d 100644 --- a/changelog.d/release-writer.added.md +++ b/changelog.d/release-writer.added.md @@ -1 +1 @@ -Add the release writer and the interface's `publish_release`, `promote_build`, `withdraw_build` and `release_receipt` calls. Each build is written to `releases///` with an updated `releases.json` in one commit pinned to `main`'s head, re-applied after a parent-commit conflict, and checked against the shared rules: no new tags, no `latest*.json` files, no change to an existing build folder, and no change of `current` while any `latest*.json` file exists. Nothing calls the writer yet. +Add the release writer and the interface's `publish_release`, `promote_build`, `withdraw_build` and `release_receipt` calls. Each build is written to `releases///` with an updated `releases.json` in one commit pinned to `main`'s head, re-applied after a parent-commit conflict, and checked against the shared rules: no new tags, no `latest*.json` files, no change to an existing build folder, and no change of `current` while any `latest*.json` file exists. Each request carries a `request_id`, so a repeated change is applied again while a resent request is applied once. Nothing calls the writer yet. diff --git a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-promote-build.json b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-promote-build.json index e2eb6760..a7b3c61d 100644 --- a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-promote-build.json +++ b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-promote-build.json @@ -6,6 +6,7 @@ "body": { "protocol_version": 1, "command": "promote_build", + "request_id": "fixture-request", "repository": "policyengine/fixture-private", "release": "fixture-release", "build": "fixture-build" diff --git a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-publish-release.json b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-publish-release.json index 0ea59b4c..f624b7fd 100644 --- a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-publish-release.json +++ b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-publish-release.json @@ -11,6 +11,7 @@ "body": { "protocol_version": 1, "command": "publish_release", + "request_id": "fixture-request", "repository": "policyengine/fixture-private", "release": "fixture-release", "build": "fixture-build", diff --git a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-withdraw-build.json b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-withdraw-build.json index e85f68fe..9a1558e3 100644 --- a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-withdraw-build.json +++ b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/fixtures/protocol-v1/huggingface-withdraw-build.json @@ -6,6 +6,7 @@ "body": { "protocol_version": 1, "command": "withdraw_build", + "request_id": "fixture-request", "repository": "policyengine/fixture-private", "release": "fixture-release", "build": "fixture-build" diff --git a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/publication.py b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/publication.py index 143c8118..990462a3 100644 --- a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/publication.py +++ b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/publication.py @@ -11,11 +11,19 @@ ``published``, is still ``pending`` (retried from the emitter's queue) or was ``refused`` by a rule. Constructing a request that breaks the shape rules raises ``ValueError``. + +Every request carries a ``request_id``, a new random one unless the caller +gives one. The emitter applies each request once: sending the same request +again (for example after its receipt was lost) returns that request's +receipt, while a new request is applied even if it repeats an earlier change, +such as promoting a build back after another was promoted, or retrying a +withdrawal or a refused change once its cause is gone. """ from __future__ import annotations import re +import uuid from collections.abc import Sequence from dataclasses import dataclass, field from pathlib import Path @@ -64,10 +72,18 @@ def __post_init__(self) -> None: raise ValueError("bytes must be a nonnegative integer.") -def _check_reference(repository: str, release: str, build: str) -> None: +def new_request_id() -> str: + """A fresh identifier for one release change request.""" + return uuid.uuid4().hex + + +def _check_reference( + repository: str, release: str, build: str, request_id: str +) -> None: repository_identifier(repository) safe_identifier(release, label="release") safe_identifier(build, label="build") + safe_identifier(request_id, label="request_id") @dataclass(frozen=True) @@ -86,10 +102,11 @@ class PublishRelease: release_role: str make_current: bool files: Sequence[ReleaseFile] + request_id: str = field(default_factory=new_request_id) protocol_version: int = field(default=PROTOCOL_VERSION, repr=False) def __post_init__(self) -> None: - _check_reference(self.repository, self.release, self.build) + _check_reference(self.repository, self.release, self.build, self.request_id) role_identifier(self.release_role, label="release_role") if not isinstance(self.make_current, bool): raise ValueError("make_current must be a boolean.") @@ -112,6 +129,7 @@ def message(self) -> dict[str, Any]: return { "protocol_version": self.protocol_version, "command": "publish_release", + "request_id": self.request_id, "repository": self.repository, "release": self.release, "build": self.build, @@ -140,15 +158,17 @@ class PromoteBuild: repository: str release: str build: str + request_id: str = field(default_factory=new_request_id) protocol_version: int = field(default=PROTOCOL_VERSION, repr=False) def __post_init__(self) -> None: - _check_reference(self.repository, self.release, self.build) + _check_reference(self.repository, self.release, self.build, self.request_id) def message(self) -> dict[str, Any]: return { "protocol_version": self.protocol_version, "command": "promote_build", + "request_id": self.request_id, "repository": self.repository, "release": self.release, "build": self.build, @@ -165,15 +185,17 @@ class WithdrawBuild: repository: str release: str build: str + request_id: str = field(default_factory=new_request_id) protocol_version: int = field(default=PROTOCOL_VERSION, repr=False) def __post_init__(self) -> None: - _check_reference(self.repository, self.release, self.build) + _check_reference(self.repository, self.release, self.build, self.request_id) def message(self) -> dict[str, Any]: return { "protocol_version": self.protocol_version, "command": "withdraw_build", + "request_id": self.request_id, "repository": self.repository, "release": self.release, "build": self.build, diff --git a/src/microcosm_emitter/huggingface/service/requests.py b/src/microcosm_emitter/huggingface/service/requests.py index 12f637a5..97caa486 100644 --- a/src/microcosm_emitter/huggingface/service/requests.py +++ b/src/microcosm_emitter/huggingface/service/requests.py @@ -275,6 +275,7 @@ def parse_release_change(message: Mapping[str, Any]) -> ReleaseHandOver: build=message.get("build"), release_role=message.get("release_role"), make_current=message.get("make_current"), + request_id=message.get("request_id"), files=tuple( ReleaseFile( name=entry.get("name"), @@ -292,6 +293,7 @@ def parse_release_change(message: Mapping[str, Any]) -> ReleaseHandOver: repository=message.get("repository"), release=message.get("release"), build=message.get("build"), + request_id=message.get("request_id"), ) else: raise RequestRefusedError("INVALID_REQUEST") diff --git a/tests/huggingface/test_release_writer.py b/tests/huggingface/test_release_writer.py index ad09d715..3a9eafb0 100644 --- a/tests/huggingface/test_release_writer.py +++ b/tests/huggingface/test_release_writer.py @@ -403,3 +403,98 @@ def test_the_module_writer_uses_the_injected_policy(tmp_path, hub): finally: module.close(0) assert (receipt["status"], receipt["error_code"]) == ("refused", "NOT_AUTHORIZED") + + +def queue_change(controls, request): + receipt = controls.module.query(request.message())["receipt"] + controls.module.delivery.run_cycle() + controls.clock.advance(1) + return controls.module.query( + { + "protocol_version": 1, + "command": "handover_receipt", + "handover_id": receipt["handover_id"], + } + )["receipt"] + + +def current_of(controls): + return ReleasesRecord.parse( + controls.hub.files(PRIVATE_REPOSITORY)["releases.json"] + ).releases[NATIONAL] + + +def promotion(build, **options): + return PromoteBuild( + repository=PRIVATE_REPOSITORY, release=NATIONAL, build=build, **options + ) + + +def withdrawal(build, **options): + return WithdrawBuild( + repository=PRIVATE_REPOSITORY, release=NATIONAL, build=build, **options + ) + + +def publish_both(controls): + for build in (FIRST_BUILD, SECOND_BUILD): + request, _ = publish_request( + controls.tmp_path, build, make_current=build == FIRST_BUILD + ) + assert queue_change(controls, request)["status"] == "published" + + +def test_promoting_back_to_an_earlier_build_applies(hugging_face_module): + controls = hugging_face_module + publish_both(controls) + # B, then A, then B again: the second promotion of B is a new request. + for build in (SECOND_BUILD, FIRST_BUILD, SECOND_BUILD): + assert queue_change(controls, promotion(build))["status"] == "published" + assert current_of(controls).current == build + + +def test_a_withdrawal_retried_after_another_promotion_applies(hugging_face_module): + controls = hugging_face_module + publish_both(controls) + refused = queue_change(controls, withdrawal(FIRST_BUILD)) + assert (refused["status"], refused["error_code"]) == ( + "refused", + "WITHDRAWING_CURRENT_BUILD", + ) + assert queue_change(controls, promotion(SECOND_BUILD))["status"] == "published" + assert queue_change(controls, withdrawal(FIRST_BUILD))["status"] == "published" + assert current_of(controls).builds[FIRST_BUILD].withdrawn is True + + +def test_a_refused_change_applies_when_requested_again_after_its_cause_is_gone( + hugging_face_module, +): + controls = hugging_face_module + publish_both(controls) + controls.hub.write_directly(PRIVATE_REPOSITORY, {"latest-national.json": b"{}"}) + refused = queue_change(controls, promotion(SECOND_BUILD)) + assert refused["error_code"] == "POINTER_FILES_PRESENT" + controls.hub.delete_directly(PRIVATE_REPOSITORY, ["latest-national.json"]) + assert queue_change(controls, promotion(SECOND_BUILD))["status"] == "published" + assert current_of(controls).current == SECOND_BUILD + + +def test_resending_the_same_request_is_a_retry_and_is_applied_once( + hugging_face_module, +): + controls = hugging_face_module + publish_both(controls) + request = promotion(SECOND_BUILD) + first = queue_change(controls, request) + commits = len(controls.hub.commits(PRIVATE_REPOSITORY)) + again = queue_change(controls, request) + assert again == first + assert len(controls.hub.commits(PRIVATE_REPOSITORY)) == commits + + +def test_a_release_change_without_a_request_id_is_refused(hugging_face_module): + controls = hugging_face_module + message = promotion(FIRST_BUILD).message() + del message["request_id"] + receipt = controls.module.query(message)["receipt"] + assert (receipt["status"], receipt["error_code"]) == ("refused", "INVALID_REQUEST") diff --git a/tests/interface/test_client_messages.py b/tests/interface/test_client_messages.py index 5a264429..5cfb370d 100644 --- a/tests/interface/test_client_messages.py +++ b/tests/interface/test_client_messages.py @@ -42,6 +42,7 @@ def fixture_publication(directory: Path) -> PublishRelease: build="fixture-build", release_role="national_default", make_current=True, + request_id="fixture-request", files=tuple( ReleaseFile( name=entry["name"], @@ -255,6 +256,7 @@ def fixtures_by_name() -> dict[str, ProtocolFixture]: repository=FIXTURE_PRIVATE_REPOSITORY, release="fixture-release", build="fixture-build", + request_id="fixture-request", ), wait_seconds=0, ), @@ -263,6 +265,7 @@ def fixtures_by_name() -> dict[str, ProtocolFixture]: repository=FIXTURE_PRIVATE_REPOSITORY, release="fixture-release", build="fixture-build", + request_id="fixture-request", ), wait_seconds=0, ), diff --git a/tests/interface/test_protocol_fixtures.py b/tests/interface/test_protocol_fixtures.py index df87ed7d..77d111cd 100644 --- a/tests/interface/test_protocol_fixtures.py +++ b/tests/interface/test_protocol_fixtures.py @@ -38,13 +38,13 @@ "68fa81b4116852f91f3c0dab3716a1974a8150039fb2b238f79efc094d9b0f6f" ), "protocol-v1/huggingface-promote-build.json": ( - "ad6bddbb09742bf45bf2d64c6c605c7641d28194e74b376c42a7872318d5b2bf" + "069248aa33b24c937e18e00bc46dd1e39c6e06febedb5f7b5cab4c1e1fea9f07" ), "protocol-v1/huggingface-publish-release.json": ( - "c7733eae51dd38f140cd2949583d86119adef38ff943db5b0e5ef1038441dc61" + "12ccf6c9b613dae4e01709b745e8caacfbeb8f2ceeb6b8bfc8373af134de25c8" ), "protocol-v1/huggingface-withdraw-build.json": ( - "b02ff56e8697ec7cc6e7658699794bb80d8f61504d970f78f9fdac19b823893c" + "ad15257013d831c5830818668b222a12c0c8b53c5236ac0ff9733bbfd76c98be" ), "protocol-v1/huggingface-check-write-access.json": ( "750b7eb8c7c9ea4112bb60560522d3b1591c6c9838239a5a54d72f90c095a631" diff --git a/tests/interface/test_release_client.py b/tests/interface/test_release_client.py index e0b8007d..d9e43812 100644 --- a/tests/interface/test_release_client.py +++ b/tests/interface/test_release_client.py @@ -123,10 +123,16 @@ def test_publication_message_names_release_build_and_each_file_role(tmp_path): ("release_manifest.json", "release_manifest"), ("microcosm_uk_2024_25.h5", "dataset"), ] + # The hand-over identity follows the request, not only its content. + resent = publication(tmp_path, make_current=True, request_id=request.request_id) + assert request.handover_id() == resent.handover_id() assert ( - request.handover_id() == publication(tmp_path, make_current=True).handover_id() + request.handover_id() != publication(tmp_path, make_current=True).handover_id() + ) + assert ( + request.handover_id() + != publication(tmp_path, request_id=request.request_id).handover_id() ) - assert request.handover_id() != publication(tmp_path).handover_id() def test_client_publishes_promotes_and_withdraws_through_the_emitter( @@ -175,3 +181,47 @@ def test_client_without_the_module_reports_unavailable(tmp_path, in_thread_emitt RELEASE, BUILD, ) + + +@pytest.mark.parametrize("change", [PromoteBuild, WithdrawBuild]) +def test_each_request_has_its_own_request_id_unless_one_is_given(change, tmp_path): + """A new request object is a new request; resending one is a retry.""" + first = change(repository=PRIVATE_REPOSITORY, release=RELEASE, build=BUILD) + second = change(repository=PRIVATE_REPOSITORY, release=RELEASE, build=BUILD) + assert first.request_id != second.request_id + assert first.handover_id() != second.handover_id() + assert first.message()["request_id"] == first.request_id + given = change( + repository=PRIVATE_REPOSITORY, + release=RELEASE, + build=BUILD, + request_id="operator-request-1", + ) + assert given.message()["request_id"] == "operator-request-1" + assert ( + given.handover_id() + == change( + repository=PRIVATE_REPOSITORY, + release=RELEASE, + build=BUILD, + request_id="operator-request-1", + ).handover_id() + ) + + +def test_publications_carry_a_request_id(tmp_path): + first = publication(tmp_path / "first") + second = publication(tmp_path / "first") + assert first.request_id != second.request_id + assert first.message()["request_id"] == first.request_id + + +@pytest.mark.parametrize("request_id", ["", "../escape", "two words", 7]) +def test_a_request_id_must_be_a_safe_identifier(request_id): + with pytest.raises(ValueError): + PromoteBuild( + repository=PRIVATE_REPOSITORY, + release=RELEASE, + build=BUILD, + request_id=request_id, + ) diff --git a/tests/support/fake_hugging_face.py b/tests/support/fake_hugging_face.py index a80ad078..77b0c520 100644 --- a/tests/support/fake_hugging_face.py +++ b/tests/support/fake_hugging_face.py @@ -122,6 +122,24 @@ def write_directly( repository, {**head.files, **files}, message, parent=head.oid ) + def delete_directly( + self, repository: str, paths: Iterable[str], message: str = "Delete" + ) -> str: + """Another writer's commit that deletes ``paths``.""" + with self._lock: + head = self.repositories[repository].head + removed = set(paths) + return self._append_commit( + repository, + { + path: content + for path, content in head.files.items() + if path not in removed + }, + message, + parent=head.oid, + ) + # HfApi surface ------------------------------------------------------- def _record(self, method: str, **arguments: Any) -> None: From ab2251eb177f0a74d9dbe4b1284cad5934a78e46 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 20 Oct 2026 17:21:23 +0400 Subject: [PATCH 05/24] Report a landed publication as published after another promotion `_already_published` refused a publication with `BUILD_FOLDER_EXISTS` when `make_current` was true and its build was listed with an identical manifest but was no longer current. That is what a retry finds after a commit that landed but raised an error, when another build was promoted in between. A build listed with a byte-identical `release_manifest.json` now always reports `published` with the commit that wrote it; when it is not current the receipt's message names the current build. Publishing never promotes an already published build. The fake Hub can now let a commit land and then raise. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- .../huggingface/service/releases.py | 20 ++++++++----- tests/huggingface/test_release_writer.py | 28 +++++++++++++++++++ tests/support/fake_hugging_face.py | 7 +++++ 4 files changed, 49 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 29e2301d..61314fb4 100644 --- a/README.md +++ b/README.md @@ -115,7 +115,7 @@ releases///... the build's files, with their rol - The writer refuses a change unless: every listed build has its folder and a `release_manifest.json` naming the same release and build; `current` names a listed build that is not withdrawn; existing build entries change only `withdrawn`; and no file inside an existing build folder is added, changed or removed (so publishing refuses a build whose folder exists). It never writes a `latest*.json` file, and refuses to change `current` while any `latest*.json` file still exists, until the old pointer files are deleted. - It creates no Hugging Face tags or branches, no `current/` folder and no mutable root file, and leaves existing tags and files untouched. It does not port `release.py`'s staging branch, release tag or pointer-file commit, which this layout replaces. - Each request carries a `request_id`, a new random one unless the caller gives one, and the emitter applies each request once. Sending the same request again returns its receipt; a new request is applied even if it repeats an earlier change (promoting a build back after another was promoted, or retrying a withdrawal or a refused change once its cause is gone). -- A retried publication whose earlier attempt already landed reports `published`. Hub failures stay `pending` and are retried; broken rules are `refused` with the rule's code. +- A publication of a build already listed with a byte-identical `release_manifest.json` reports `published` with the commit that wrote it, as when a retried attempt finds that its earlier commit landed, even if another build has been made current since. It never makes that build current; `promote_build` does. Hub failures stay `pending` and are retried; broken rules are `refused` with the rule's code. - `release_change_policy` is where a decision on who may promote or withdraw will plug in; by default every change is allowed. `current` may stay `null`, for a release that is never promoted. ## Development diff --git a/src/microcosm_emitter/huggingface/service/releases.py b/src/microcosm_emitter/huggingface/service/releases.py index 6fb5c64a..8b913c7b 100644 --- a/src/microcosm_emitter/huggingface/service/releases.py +++ b/src/microcosm_emitter/huggingface/service/releases.py @@ -224,11 +224,13 @@ def _already_published( record: ReleasesRecord, head: str, ) -> ReleaseOutcome: - """A build folder that exists is refused, unless this request made it. + """A build folder that exists is refused, unless this build landed. - A retried publication whose earlier attempt landed (the receipt was lost) - finds its own build listed with byte-identical manifest: that is - published, not refused. + A retried publication whose earlier attempt landed (its commit raised, or + the receipt was lost) finds its build listed with a byte-identical + manifest: that is ``published``, not refused, even if another build has + been made current since. Publishing never promotes an already published + build; ``promote_build`` does that. """ release, build = request["release"], request["build"] entry = record.releases.get(release) @@ -241,8 +243,6 @@ def _already_published( exists_message = f"{build_folder(release, build)}/ already exists." if entry is None or build not in entry.builds: return _refused("BUILD_FOLDER_EXISTS", exists_message) - if request["make_current"] and entry.current != build: - return _refused("BUILD_FOLDER_EXISTS", exists_message) try: same = repository.download(manifest_path, revision=head) == ( local_manifest.read_bytes() @@ -252,4 +252,10 @@ def _already_published( return _pending("REPOSITORY_UNAVAILABLE") if not same: return _refused("BUILD_FOLDER_EXISTS", exists_message) - return ReleaseOutcome("published", commit=commit or head) + message = None + if request["make_current"] and entry.current != build: + message = ( + f"Build {build} of {release} was already published; the current " + f"build is {entry.current}." + ) + return ReleaseOutcome("published", message=message, commit=commit or head) diff --git a/tests/huggingface/test_release_writer.py b/tests/huggingface/test_release_writer.py index 3a9eafb0..e9e715aa 100644 --- a/tests/huggingface/test_release_writer.py +++ b/tests/huggingface/test_release_writer.py @@ -498,3 +498,31 @@ def test_a_release_change_without_a_request_id_is_refused(hugging_face_module): del message["request_id"] receipt = controls.module.query(message)["receipt"] assert (receipt["status"], receipt["error_code"]) == ("refused", "INVALID_REQUEST") + + +def test_a_landed_publication_retried_after_another_promotion_is_published( + hub, tmp_path +): + """The commit landed but raised; another build was promoted meanwhile.""" + request, _ = publish_request(tmp_path, FIRST_BUILD, make_current=True) + hub.fail_after_next_commit(FakeHubHTTPError(503)) + first = apply(hub, request) + assert (first.status, first.error_code) == ("pending", "UPLOAD_FAILED") + landed = hub.commits(PRIVATE_REPOSITORY)[-1] + other, _ = publish_request(tmp_path, SECOND_BUILD, make_current=True) + assert apply(hub, other).status == "published" + retried = apply(hub, request, published_at="2026-10-21T00:00:00+00:00") + assert (retried.status, retried.commit) == ("published", landed.oid) + assert record_of(hub).releases[NATIONAL].current == SECOND_BUILD + assert len(hub.commits(PRIVATE_REPOSITORY)) == 2 + + +def test_republishing_a_published_build_does_not_promote_it(hub, tmp_path): + """Promoting is ``promote_build``'s job; publication reports what landed.""" + listed, _ = publish_request(tmp_path, FIRST_BUILD) + assert apply(hub, listed).status == "published" + again, _ = publish_request(tmp_path, FIRST_BUILD, make_current=True) + outcome = apply(hub, again) + assert outcome.status == "published" + assert record_of(hub).releases[NATIONAL].current is None + assert len(hub.commits(PRIVATE_REPOSITORY)) == 1 diff --git a/tests/support/fake_hugging_face.py b/tests/support/fake_hugging_face.py index 77b0c520..3b216c5c 100644 --- a/tests/support/fake_hugging_face.py +++ b/tests/support/fake_hugging_face.py @@ -83,6 +83,7 @@ def __init__(self, *, whoami: Mapping[str, Any] | None = None) -> None: self.whoami_response = whoami self._failures: dict[str, list[BaseException]] = {} self._before_commit: list[Callable[[], None]] = [] + self._after_commit_failures: list[BaseException] = [] self._lock = threading.RLock() self._download_directory = Path(tempfile.mkdtemp(prefix="fake-hub-")) self._sequence = 0 @@ -102,6 +103,10 @@ def fail_next(self, method: str, error: BaseException, *, times: int = 1) -> Non """Raise ``error`` from the next ``times`` calls of ``method``.""" self._failures.setdefault(method, []).extend([error] * times) + def fail_after_next_commit(self, error: BaseException) -> None: + """Let the next commit land, then raise ``error`` as if it had not.""" + self._after_commit_failures.append(error) + def run_before_next_commit(self, action: Callable[[], None]) -> None: """Run ``action`` (for example another writer's commit) first.""" self._before_commit.append(action) @@ -227,6 +232,8 @@ def create_commit( parent=head.oid, added=added, ) + if self._after_commit_failures: + raise self._after_commit_failures.pop(0) return SimpleNamespace(oid=oid, commit_url=f"https://fake/{oid}") def hf_hub_download( From 6642d387de7be01556b291d5ccb7de136e02486d Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 20 Oct 2026 17:22:35 +0400 Subject: [PATCH 06/24] Refuse deeply nested or oversized release records instead of retrying `ReleasesRecord.parse` caught only `ValueError` and `UnicodeDecodeError`, and `check_repository` likewise. A deeply nested `releases.json` or release manifest makes `json.loads` raise `RecursionError`, which `apply_release_change` mapped to a pending `REPOSITORY_UNAVAILABLE` and retried forever. - Such a record is refused as `INVALID_RECORD`, and such a manifest as `MANIFEST_MISMATCH`; both are final. - A record larger than `MAX_RECORD_BYTES` (4 MiB) is refused as `INVALID_RECORD` before it is decoded. Co-Authored-By: Claude Opus 5.5 --- changelog.d/releases-record-checks.added.md | 2 +- .../microcosm_emitter_interface/releases.py | 22 +++++++-- tests/huggingface/test_release_writer.py | 46 +++++++++++++++++++ tests/interface/test_releases_record.py | 32 +++++++++++++ 4 files changed, 97 insertions(+), 5 deletions(-) diff --git a/changelog.d/releases-record-checks.added.md b/changelog.d/releases-record-checks.added.md index 8d0ba82c..82959065 100644 --- a/changelog.d/releases-record-checks.added.md +++ b/changelog.d/releases-record-checks.added.md @@ -1 +1 @@ -Add `microcosm_emitter_interface.releases`: the shape of each dataset repository's `releases.json`, the changes the release writer makes (publish a build, make a build current, withdraw a build), and the checks every write must pass, so Microcosm and the emitter run the same code. +Add `microcosm_emitter_interface.releases`: the shape of each dataset repository's `releases.json`, the changes the release writer makes (publish a build, make a build current, withdraw a build), and the checks every write must pass, so Microcosm and the emitter run the same code. A `releases.json` above 4 MiB, or nested too deeply to decode, is refused as invalid, and so is a manifest nested too deeply. diff --git a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/releases.py b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/releases.py index 232e5330..1f35737e 100644 --- a/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/releases.py +++ b/packages/microcosm-emitter-interface/src/microcosm_emitter_interface/releases.py @@ -39,6 +39,12 @@ #: ``latest-evidence.json``, ...). No write may create one, and ``current`` #: may not change while any still exists. POINTER_FILE_PATTERN: Final = "latest*.json" +#: The largest ``releases.json`` accepted. A record lists a few hundred bytes +#: per build, so this leaves room for thousands of builds. +MAX_RECORD_BYTES: Final = 4 * 1024 * 1024 +#: What ``json.loads`` raises for bytes that are not a usable JSON document: +#: invalid JSON or text, and nesting too deep to decode (``RecursionError``). +_UNREADABLE_JSON_ERRORS: Final = (ValueError, UnicodeDecodeError, RecursionError) _RECORD_KEYS: Final = {"schema_version", "default", "releases"} _RELEASE_KEYS: Final = {"role", "current", "builds"} @@ -91,10 +97,17 @@ def empty(cls) -> ReleasesRecord: @classmethod def parse(cls, content: bytes) -> ReleasesRecord: + if len(content) > MAX_RECORD_BYTES: + raise _refuse( + "INVALID_RECORD", + f"releases.json is larger than {MAX_RECORD_BYTES} bytes.", + ) try: document = json.loads(content) - except (ValueError, UnicodeDecodeError): - raise _refuse("INVALID_RECORD", "releases.json is not JSON.") from None + except _UNREADABLE_JSON_ERRORS: + raise _refuse( + "INVALID_RECORD", "releases.json is not a readable JSON document." + ) from None record = cls._from_document(document) violations = check_record(record) if violations: @@ -345,9 +358,10 @@ def check_repository( Violation("MANIFEST_MISSING", f"{manifest_path} is missing.") ) continue + content = read_file(manifest_path) try: - manifest = json.loads(read_file(manifest_path)) - except (ValueError, UnicodeDecodeError): + manifest = json.loads(content) + except _UNREADABLE_JSON_ERRORS: manifest = None if ( not isinstance(manifest, dict) diff --git a/tests/huggingface/test_release_writer.py b/tests/huggingface/test_release_writer.py index e9e715aa..0ca0ed75 100644 --- a/tests/huggingface/test_release_writer.py +++ b/tests/huggingface/test_release_writer.py @@ -526,3 +526,49 @@ def test_republishing_a_published_build_does_not_promote_it(hub, tmp_path): assert outcome.status == "published" assert record_of(hub).releases[NATIONAL].current is None assert len(hub.commits(PRIVATE_REPOSITORY)) == 1 + + +DEEPLY_NESTED = b"[" * 100_000 + b"]" * 100_000 + + +def test_a_deeply_nested_record_in_the_repository_is_refused_not_retried(hub, tmp_path): + hub.write_directly(PRIVATE_REPOSITORY, {"releases.json": DEEPLY_NESTED}) + request, _ = publish_request(tmp_path, FIRST_BUILD) + outcome = apply(hub, request) + assert (outcome.status, outcome.error_code, outcome.retry) == ( + "refused", + "INVALID_RECORD", + False, + ) + + +def test_a_deeply_nested_local_manifest_is_refused_not_retried(hub, tmp_path): + request, _ = publish_request(tmp_path, FIRST_BUILD) + manifest = next(item for item in request.files if item.role == "release_manifest") + manifest.local_path.write_bytes(DEEPLY_NESTED) + nested = PublishRelease( + repository=request.repository, + release=request.release, + build=request.build, + release_role=request.release_role, + make_current=False, + files=tuple( + ReleaseFile( + name=item.name, + role=item.role, + local_path=item.local_path, + sha256=hashlib.sha256(DEEPLY_NESTED).hexdigest(), + bytes=len(DEEPLY_NESTED), + ) + if item is manifest + else item + for item in request.files + ), + ) + outcome = apply(hub, nested) + assert (outcome.status, outcome.error_code, outcome.retry) == ( + "refused", + "MANIFEST_MISMATCH", + False, + ) + assert hub.commits(PRIVATE_REPOSITORY) == [] diff --git a/tests/interface/test_releases_record.py b/tests/interface/test_releases_record.py index c7afc13d..ed524c3c 100644 --- a/tests/interface/test_releases_record.py +++ b/tests/interface/test_releases_record.py @@ -366,3 +366,35 @@ def test_current_cannot_change_while_pointer_files_exist(): after=first_record_with_current, ) ) == {"POINTER_FILES_PRESENT"} + + +DEEPLY_NESTED = b"[" * 100_000 + b"]" * 100_000 + + +def test_a_deeply_nested_record_is_refused_as_invalid(): + """``json.loads`` raises ``RecursionError`` for it, not ``ValueError``.""" + with pytest.raises(ReleaseRecordError) as refusal: + ReleasesRecord.parse(DEEPLY_NESTED) + assert codes(refusal.value.violations) == {"INVALID_RECORD"} + + +def test_a_record_above_the_size_limit_is_refused_as_invalid(): + from microcosm_emitter_interface.releases import MAX_RECORD_BYTES + + padded = json.dumps( + {**DOCUMENTED_EXAMPLE, "padding": "x" * MAX_RECORD_BYTES} + ).encode() + with pytest.raises(ReleaseRecordError) as refusal: + ReleasesRecord.parse(padded) + assert codes(refusal.value.violations) == {"INVALID_RECORD"} + assert "bytes" in refusal.value.violations[0].message + + +def test_a_deeply_nested_manifest_does_not_name_its_build(): + record = documented_record() + files = repository_files(record) + newer_folder = build_folder(NATIONAL, NEWER_BUILD) + nested = {**files, f"{newer_folder}/release_manifest.json": DEEPLY_NESTED} + assert codes(check_repository(record, nested, nested.__getitem__)) == { + "MANIFEST_MISMATCH" + } From 0659feacbe2e5ea7a9ac17d53be29194321cf56d Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 20 Oct 2026 17:24:13 +0400 Subject: [PATCH 07/24] Keep the commit spacing for every job except release changes The release writer's change to `HandoverQueue.due_repositories` let every job skip `MIN_COMMIT_SPACING_SECONDS`, not only release changes, so bundles could again commit in bursts, each starting the dataset repository's webhook. Only jobs of `UNSPACED_JOB_KINDS` (release changes) now skip the spacing; a repository due only for a bundle waits, and within the spacing the worker claims only a release change, so an earlier bundle job is not taken in its place. Co-Authored-By: Claude Opus 5.5 --- README.md | 1 + .../huggingface/service/delivery.py | 17 +++- .../huggingface/service/queue.py | 35 ++++++-- tests/huggingface/test_release_writer.py | 82 +++++++++++++++++++ 4 files changed, 125 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 61314fb4..fd81f0d4 100644 --- a/README.md +++ b/README.md @@ -116,6 +116,7 @@ releases///... the build's files, with their rol - It creates no Hugging Face tags or branches, no `current/` folder and no mutable root file, and leaves existing tags and files untouched. It does not port `release.py`'s staging branch, release tag or pointer-file commit, which this layout replaces. - Each request carries a `request_id`, a new random one unless the caller gives one, and the emitter applies each request once. Sending the same request again returns its receipt; a new request is applied even if it repeats an earlier change (promoting a build back after another was promoted, or retrying a withdrawal or a refused change once its cause is gone). - A publication of a build already listed with a byte-identical `release_manifest.json` reports `published` with the commit that wrote it, as when a retried attempt finds that its earlier commit landed, even if another build has been made current since. It never makes that build current; `promote_build` does. Hub failures stay `pending` and are retried; broken rules are `refused` with the rule's code. +- A release change goes at the next cycle, even within 10 seconds of this machine's last commit to the repository; bundles and run files keep that spacing. Each cycle still makes at most one commit per repository. - `release_change_policy` is where a decision on who may promote or withdraw will plug in; by default every change is allowed. `current` may stay `null`, for a release that is never promoted. ## Development diff --git a/src/microcosm_emitter/huggingface/service/delivery.py b/src/microcosm_emitter/huggingface/service/delivery.py index 0d1c1e29..9aab517a 100644 --- a/src/microcosm_emitter/huggingface/service/delivery.py +++ b/src/microcosm_emitter/huggingface/service/delivery.py @@ -39,6 +39,9 @@ DATASET_BUNDLE = "dataset_bundle" RELEASE_CHANGE = "release_change" SUPPORTED_JOB_KINDS = frozenset({DATASET_BUNDLE, RELEASE_CHANGE}) +#: Job kinds that skip the spacing between commits: release changes are rare +#: and a build waits for them. Bundles and run files keep it. +UNSPACED_JOB_KINDS = frozenset({RELEASE_CHANGE}) CREDENTIAL_MISSING = "CREDENTIAL_MISSING" UPLOAD_FAILED = "UPLOAD_FAILED" CREDENTIAL_MISSING_MESSAGE = ( @@ -195,7 +198,10 @@ def _cycle( stop_requested: Callable[[], bool] = never_stop, ) -> int: repositories = self.queue.due_repositories( - self.clock(), kinds=job_kinds, run_files=run_files + self.clock(), + kinds=job_kinds, + unspaced_kinds=UNSPACED_JOB_KINDS, + run_files=run_files, ) if not repositories: return 0 @@ -294,9 +300,16 @@ def _run_files_to_commit( ) def _claim_job(self, repository: str, kinds: frozenset[str]) -> ClaimedJob | None: + """The next due job of ``kinds``; within the commit spacing, only a + release change.""" + now = self.clock() + if not self.queue.commit_spacing_elapsed(repository, now): + kinds = kinds & UNSPACED_JOB_KINDS + if not kinds: + return None return self.queue.claim_job( repository, - now=self.clock(), + now=now, lease_seconds=REPOSITORY_LEASE_SECONDS, kinds=kinds, ) diff --git a/src/microcosm_emitter/huggingface/service/queue.py b/src/microcosm_emitter/huggingface/service/queue.py index eb9fd49f..ce79b79e 100644 --- a/src/microcosm_emitter/huggingface/service/queue.py +++ b/src/microcosm_emitter/huggingface/service/queue.py @@ -198,6 +198,7 @@ def due_repositories( now: float, *, kinds: Iterable[str] | None = None, + unspaced_kinds: Iterable[str] = (), run_files: bool = True, ) -> list[str]: """Repositories with work due now that may receive a commit now. @@ -205,8 +206,11 @@ def due_repositories( ``kinds`` limits jobs to those this emitter can deliver, so a job queued by a newer emitter sharing the database waits for it; an empty ``kinds`` leaves jobs out. ``run_files`` false leaves out run files. + Every commit waits ``MIN_COMMIT_SPACING_SECONDS`` after this machine's + last commit to the repository, except jobs of ``unspaced_kinds``. """ job_kinds = None if kinds is None else list(kinds) + unspaced = set(unspaced_kinds) with self._sessions() as session: run_file_repositories: set[str] = set() if run_files: @@ -225,9 +229,10 @@ def due_repositories( ) ) ) - job_repositories: set[str] = set() + spaced_job_repositories: set[str] = set() + unspaced_job_repositories: set[str] = set() if job_kinds != []: - job_query = select(HandoverJob.target).where( + job_query = select(HandoverJob.target, HandoverJob.kind).where( HandoverJob.destination == DESTINATION_HUGGING_FACE, HandoverJob.status == JOB_PENDING, HandoverJob.next_attempt_at <= now, @@ -235,15 +240,28 @@ def due_repositories( ) if job_kinds is not None: job_query = job_query.where(HandoverJob.kind.in_(job_kinds)) - job_repositories = set(session.scalars(job_query)) + for target, kind in session.execute(job_query): + if kind in unspaced: + unspaced_job_repositories.add(target) + else: + spaced_job_repositories.add(target) due = [] - for repository in sorted(run_file_repositories | job_repositories): + for repository in sorted( + run_file_repositories + | spaced_job_repositories + | unspaced_job_repositories + ): schedule = session.get(RepositoryCommitSchedule, repository) if schedule is not None and schedule.lease_until > now: continue - if repository in job_repositories or ( - repository in run_file_repositories - and self._run_files_allowed(schedule, now) + spaced = self._spacing_elapsed(schedule, now) + if ( + repository in unspaced_job_repositories + or (repository in spaced_job_repositories and spaced) + or ( + repository in run_file_repositories + and self._run_files_allowed(schedule, now) + ) ): due.append(repository) return due @@ -253,7 +271,8 @@ def _spacing_elapsed(schedule: RepositoryCommitSchedule | None, now: float) -> b """Whether this machine's last commit to the repository is far enough back. The spacing keeps bursts of commits, each starting the dataset - repository's webhook, from reaching the Hub. + repository's webhook, from reaching the Hub; only release changes, + which are rare and explicit, skip it. """ return ( schedule is None diff --git a/tests/huggingface/test_release_writer.py b/tests/huggingface/test_release_writer.py index 0ca0ed75..36784d8a 100644 --- a/tests/huggingface/test_release_writer.py +++ b/tests/huggingface/test_release_writer.py @@ -572,3 +572,85 @@ def test_a_deeply_nested_local_manifest_is_refused_not_retried(hub, tmp_path): False, ) assert hub.commits(PRIVATE_REPOSITORY) == [] + + +def hand_over_private_run_file(controls): + from microcosm_emitter_interface.staging import RunFile, StageRunFiles + + local = controls.tmp_path / "run-files" / "progress.json" + local.parent.mkdir(parents=True, exist_ok=True) + local.write_bytes(b"{}") + controls.module.query( + StageRunFiles( + repository=PRIVATE_REPOSITORY, + run_id="uk-run-1", + files=(RunFile("progress.json", local),), + flush=True, + ).message() + ) + + +def bundle_request(controls): + from microcosm_emitter_interface.bundles import BundleFile, StageDatasetBundle + + directory = controls.tmp_path / "bundle" + directory.mkdir(parents=True, exist_ok=True) + files = [] + for name, role, content in ( + ("dataset.h5", "dataset", b"rows"), + ("staged_manifest.json", "staged_manifest", b'{"files": {}}'), + ): + (directory / name).write_bytes(content) + files.append( + BundleFile( + name=name, + role=role, + local_path=directory / name, + sha256=hashlib.sha256(content).hexdigest(), + bytes=len(content), + ) + ) + return StageDatasetBundle( + repository=PRIVATE_REPOSITORY, run_id="uk-run-1", files=tuple(files) + ) + + +def receipt_status(controls, handover_id): + return controls.module.query( + { + "protocol_version": 1, + "command": "handover_receipt", + "handover_id": handover_id, + } + )["receipt"]["status"] + + +def test_release_changes_skip_the_commit_spacing_but_bundles_keep_it( + hugging_face_module, +): + from microcosm_emitter.huggingface.service.queue import ( + MIN_COMMIT_SPACING_SECONDS, + ) + + controls = hugging_face_module + hand_over_private_run_file(controls) + controls.module.delivery.run_cycle() + assert len(controls.hub.commits(PRIVATE_REPOSITORY)) == 1 + bundle = bundle_request(controls) + controls.module.query(bundle.message()) + controls.clock.advance(1) + controls.module.delivery.run_cycle() + # Within the spacing, nothing but a release change may commit. + assert len(controls.hub.commits(PRIVATE_REPOSITORY)) == 1 + assert receipt_status(controls, bundle.handover_id()) == "pending" + publication, _ = publish_request(controls.tmp_path, FIRST_BUILD) + controls.module.query(publication.message()) + controls.module.delivery.run_cycle() + assert receipt_status(controls, publication.handover_id()) == "published" + assert receipt_status(controls, bundle.handover_id()) == "pending" + controls.clock.advance(MIN_COMMIT_SPACING_SECONDS - 1) + controls.module.delivery.run_cycle() + assert receipt_status(controls, bundle.handover_id()) == "pending" + controls.clock.advance(1) + controls.module.delivery.run_cycle() + assert receipt_status(controls, bundle.handover_id()) == "uploaded" From ff52dda857d3376477cdfc3a7fdd0488d4ff50b6 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 20 Oct 2026 17:28:52 +0400 Subject: [PATCH 08/24] Queue typed release changes and pass the caller to the change policy - The module queues the validated change's own message (`PublishRelease.message()` and so on) instead of `dict(message)`, so keys the rules do not know never reach the queue or the hand-over identifier. The worker rebuilds the typed change from it (`release_change_from_message`), and `apply_release_change` takes the typed change. Release and bundle outcome statuses are `Literal` types. - `MAX_PARENT_CONFLICT_ATTEMPTS`, `UPLOAD_FAILED`, `REPOSITORY_UNAVAILABLE`, `PRESERVED_FILES_MISSING` and `PRESERVED_BYTES_CHANGED` are defined once, in `huggingface.service.hub`; `RELEASE_CHANGE` once, in the interface; file hashing once, in the queue. - The credential lookup is public (`core.auth.huggingface_token`), so the Hugging Face module no longer imports a private name. - `ReleaseChangePolicy` now receives the change and a `ReleaseCaller` (the user the emitter runs as and the build process that started it). The module applies it when the change is handed over, so a refusal (`NOT_AUTHORIZED`) is answered at once and never queued. Who may promote is still undecided; the default allows every change. Co-Authored-By: Claude Opus 5.5 --- README.md | 2 +- src/microcosm_emitter/core/auth.py | 6 +- .../huggingface/service/bundles.py | 29 ++-- .../huggingface/service/delivery.py | 28 ++-- .../huggingface/service/hub.py | 13 +- .../huggingface/service/module.py | 35 +++-- .../huggingface/service/queue.py | 9 ++ .../huggingface/service/releases.py | 143 +++++++++--------- .../huggingface/service/requests.py | 62 +++++--- .../huggingface/service/run_files.py | 2 +- .../telemetry/service/collector.py | 4 +- tests/huggingface/test_release_writer.py | 128 ++++++++++++---- tests/telemetry/test_delivery.py | 18 +-- tests/test_boundaries.py | 47 ++++++ 14 files changed, 343 insertions(+), 183 deletions(-) diff --git a/README.md b/README.md index fd81f0d4..898111ac 100644 --- a/README.md +++ b/README.md @@ -117,7 +117,7 @@ releases///... the build's files, with their rol - Each request carries a `request_id`, a new random one unless the caller gives one, and the emitter applies each request once. Sending the same request again returns its receipt; a new request is applied even if it repeats an earlier change (promoting a build back after another was promoted, or retrying a withdrawal or a refused change once its cause is gone). - A publication of a build already listed with a byte-identical `release_manifest.json` reports `published` with the commit that wrote it, as when a retried attempt finds that its earlier commit landed, even if another build has been made current since. It never makes that build current; `promote_build` does. Hub failures stay `pending` and are retried; broken rules are `refused` with the rule's code. - A release change goes at the next cycle, even within 10 seconds of this machine's last commit to the repository; bundles and run files keep that spacing. Each cycle still makes at most one commit per repository. -- `release_change_policy` is where a decision on who may promote or withdraw will plug in; by default every change is allowed. `current` may stay `null`, for a release that is never promoted. +- `release_change_policy` is where a decision on who may promote or withdraw will plug in. It receives the validated change and its caller (`ReleaseCaller`: the user the emitter runs as and the build process that started it) when the change is handed over; a refusal is answered at once as `NOT_AUTHORIZED` and never queued. By default every change is allowed. `current` may stay `null`, for a release that is never promoted. ## Development diff --git a/src/microcosm_emitter/core/auth.py b/src/microcosm_emitter/core/auth.py index 527bcf4c..44c4d47c 100644 --- a/src/microcosm_emitter/core/auth.py +++ b/src/microcosm_emitter/core/auth.py @@ -102,7 +102,9 @@ def _http_post( return error.code, _decode_response(body) -def _huggingface_token() -> str | None: +def huggingface_token() -> str | None: + """The build's Hugging Face token: `HF_TOKEN`, then `HUGGINGFACE_TOKEN`, then + the token saved by `hf auth login`; `None` without one.""" return ( os.environ.get("HF_TOKEN", "").strip() or os.environ.get("HUGGINGFACE_TOKEN", "").strip() @@ -118,7 +120,7 @@ def __init__(self, origin: str, *, post=None, credential=None): self.origin = origin self._post = post or _http_post - self._credential = credential or _huggingface_token + self._credential = credential or huggingface_token self._session_token: tuple[str, float] | None = None self._lock = threading.Lock() diff --git a/src/microcosm_emitter/huggingface/service/bundles.py b/src/microcosm_emitter/huggingface/service/bundles.py index 12422593..131a1a2c 100644 --- a/src/microcosm_emitter/huggingface/service/bundles.py +++ b/src/microcosm_emitter/huggingface/service/bundles.py @@ -9,47 +9,42 @@ from __future__ import annotations -import hashlib import json from collections.abc import Mapping from dataclasses import dataclass from pathlib import Path -from typing import Any +from typing import Any, Literal from microcosm_emitter.huggingface.service.hub import ( + MAX_PARENT_CONFLICT_ATTEMPTS, + PRESERVED_BYTES_CHANGED, + PRESERVED_FILES_MISSING, + REPOSITORY_UNAVAILABLE, + UPLOAD_FAILED, DatasetRepository, add_operation, is_parent_conflict, ) +from microcosm_emitter.huggingface.service.queue import sha256_file from microcosm_emitter_interface.bundles import BUNDLE_INVENTORY_ROLE, SIDECAR_ROLES -MAX_PARENT_CONFLICT_ATTEMPTS = 5 -REPOSITORY_UNAVAILABLE = "REPOSITORY_UNAVAILABLE" -UPLOAD_FAILED = "UPLOAD_FAILED" REMOTE_DIFFERS = "REMOTE_DIFFERS" -PRESERVED_FILES_MISSING = "PRESERVED_FILES_MISSING" -PRESERVED_BYTES_CHANGED = "PRESERVED_BYTES_CHANGED" + + +type BundleStatus = Literal["pending", "uploaded", "already_staged", "failed"] @dataclass(frozen=True) class BundleOutcome: """One attempt's result: final unless ``retry``.""" - status: str + status: BundleStatus error_code: str | None = None revision: str | None = None retry: bool = False committed: bool = False -def _sha256_file(path: Path) -> str: - digest = hashlib.sha256() - with path.open("rb") as stream: - for block in iter(lambda: stream.read(1024 * 1024), b""): - digest.update(block) - return digest.hexdigest() - - def _same_digests(remote: Any, local: Mapping[str, str]) -> bool: """Microcosm ``_same_digests``: compare the outputs' digests only.""" if not isinstance(remote, Mapping) or set(remote) != set(local): @@ -69,7 +64,7 @@ def _local_files_outcome(files: list[Mapping[str, Any]]) -> BundleOutcome | None return BundleOutcome("pending", PRESERVED_FILES_MISSING, retry=True) if ( path.stat().st_size != entry["bytes"] - or _sha256_file(path) != entry["sha256"] + or sha256_file(path) != entry["sha256"] ): return BundleOutcome("failed", PRESERVED_BYTES_CHANGED) except OSError: diff --git a/src/microcosm_emitter/huggingface/service/delivery.py b/src/microcosm_emitter/huggingface/service/delivery.py index 9aab517a..6d2dab4c 100644 --- a/src/microcosm_emitter/huggingface/service/delivery.py +++ b/src/microcosm_emitter/huggingface/service/delivery.py @@ -20,14 +20,18 @@ from typing import Any from microcosm_emitter.huggingface.service.bundles import stage_bundle -from microcosm_emitter.huggingface.service.hub import DatasetRepository +from microcosm_emitter.huggingface.service.hub import UPLOAD_FAILED, DatasetRepository from microcosm_emitter.huggingface.service.queue import ClaimedJob, HandoverQueue from microcosm_emitter.huggingface.service.releases import ( - ReleaseChangePolicy, - allow_every_change, + ReleaseOutcome, apply_release_change, ) +from microcosm_emitter.huggingface.service.requests import ( + RequestRefusedError, + release_change_from_message, +) from microcosm_emitter.huggingface.service.run_files import commit_run_files +from microcosm_emitter_interface.publication import RELEASE_CHANGE #: How long a repository or job lease lasts without renewal. While a delivery #: runs, its leases are renewed every ``LEASE_RENEWAL_SECONDS``, so a long @@ -37,13 +41,11 @@ LEASE_RENEWAL_SECONDS = 30.0 #: The job kinds this emitter delivers. DATASET_BUNDLE = "dataset_bundle" -RELEASE_CHANGE = "release_change" SUPPORTED_JOB_KINDS = frozenset({DATASET_BUNDLE, RELEASE_CHANGE}) #: Job kinds that skip the spacing between commits: release changes are rare #: and a build waits for them. Bundles and run files keep it. UNSPACED_JOB_KINDS = frozenset({RELEASE_CHANGE}) CREDENTIAL_MISSING = "CREDENTIAL_MISSING" -UPLOAD_FAILED = "UPLOAD_FAILED" CREDENTIAL_MISSING_MESSAGE = ( "Microcosm Hugging Face uploads are paused: no Hugging Face credential was " "found (HF_TOKEN, HUGGINGFACE_TOKEN or `hf auth login`). Handed-over files " @@ -141,13 +143,11 @@ def __init__( api_factory: Callable[[], Any | None], clock: Callable[[], float] = time.time, lease_renewal_seconds: float = LEASE_RENEWAL_SECONDS, - release_change_policy: ReleaseChangePolicy = allow_every_change, ) -> None: self.queue = queue self.api_factory = api_factory self.clock = clock self.lease_renewal_seconds = lease_renewal_seconds - self.release_change_policy = release_change_policy self._warned: set[str] = set() self._warning_lock = threading.Lock() @@ -353,12 +353,14 @@ def _deliver_job(self, hub: DatasetRepository, held: HeldLeases) -> bool: job = held.job assert job is not None, "The job is claimed before delivery." if job.kind == RELEASE_CHANGE: - outcome = apply_release_change( - hub, - job.request, - published_at=iso_timestamp(self.clock()), - policy=self.release_change_policy, - ) + try: + change = release_change_from_message(job.request) + except RequestRefusedError as refusal: + outcome = ReleaseOutcome("refused", refusal.code) + else: + outcome = apply_release_change( + hub, change, published_at=iso_timestamp(self.clock()) + ) revision, message = outcome.commit, outcome.message else: outcome = stage_bundle(hub, job.request, job.files) diff --git a/src/microcosm_emitter/huggingface/service/hub.py b/src/microcosm_emitter/huggingface/service/hub.py index 6db29e6c..75016a96 100644 --- a/src/microcosm_emitter/huggingface/service/hub.py +++ b/src/microcosm_emitter/huggingface/service/hub.py @@ -13,10 +13,19 @@ from pathlib import Path from typing import Any -from microcosm_emitter.core.auth import _huggingface_token +from microcosm_emitter.core.auth import huggingface_token #: HTTP statuses the Hub returns when ``parent_commit`` is no longer the head. PARENT_CONFLICT_STATUSES = frozenset({409, 412}) +#: How often a commit pinned to a head is rebuilt on the new head after +#: another writer's commit landed first. +MAX_PARENT_CONFLICT_ATTEMPTS = 5 +#: Error codes the hand-over receipts share. The first two are retried. +REPOSITORY_UNAVAILABLE = "REPOSITORY_UNAVAILABLE" +UPLOAD_FAILED = "UPLOAD_FAILED" +#: A preserved file is missing (retried) or its bytes changed (final). +PRESERVED_FILES_MISSING = "PRESERVED_FILES_MISSING" +PRESERVED_BYTES_CHANGED = "PRESERVED_BYTES_CHANGED" #: Errors huggingface_hub raises when the Hub answered that a file is missing #: (``RemoteEntryNotFoundError`` from 1.0, ``EntryNotFoundError`` before). _MISSING_FILE_ERROR_NAMES = frozenset( @@ -35,7 +44,7 @@ def hugging_face_api() -> Any | None: saved by ``hf auth login``: the same order telemetry uses. It is read again for every cycle, so a login during a long build is picked up. """ - token = _huggingface_token() + token = huggingface_token() if not token: return None from huggingface_hub import HfApi diff --git a/src/microcosm_emitter/huggingface/service/module.py b/src/microcosm_emitter/huggingface/service/module.py index df234a76..2583c720 100644 --- a/src/microcosm_emitter/huggingface/service/module.py +++ b/src/microcosm_emitter/huggingface/service/module.py @@ -12,6 +12,7 @@ from __future__ import annotations +import getpass import time from collections.abc import Callable, Mapping from pathlib import Path @@ -22,7 +23,6 @@ from microcosm_emitter.huggingface.service.delivery import ( CREDENTIAL_MISSING, DATASET_BUNDLE, - RELEASE_CHANGE, HuggingFaceDelivery, ) from microcosm_emitter.huggingface.service.hub import ( @@ -31,6 +31,7 @@ ) from microcosm_emitter.huggingface.service.queue import HandoverQueue, sha256_bytes from microcosm_emitter.huggingface.service.releases import ( + ReleaseCaller, ReleaseChangePolicy, allow_every_change, ) @@ -44,6 +45,7 @@ ) from microcosm_emitter.huggingface.service.workers import DeliveryWorker from microcosm_emitter_interface.identifiers import repository_identifier +from microcosm_emitter_interface.publication import RELEASE_CHANGE #: A read-back of a run with no file this emitter committed: nothing to compare. NO_COMMITTED_FILES = "NO_COMMITTED_FILES" @@ -108,7 +110,6 @@ def initialize(self) -> None: self.queue, api_factory=self.api_factory, clock=self.clock, - release_change_policy=self.release_change_policy, ) if self.background_workers: self.workers = self._delivery_workers(self.delivery) @@ -210,22 +211,36 @@ def _stage_dataset_bundle(self, message: JsonObject) -> dict[str, Any]: files=hand_over.copied, ) + def _caller(self) -> ReleaseCaller: + return ReleaseCaller(user=getpass.getuser(), parent_pid=self.context.parent_pid) + def _release_change(self, message: JsonObject) -> dict[str, Any]: assert self.queue is not None - try: - hand_over = parse_release_change(message) - except RequestRefusedError as refusal: + + def refused( + code: str, handover_id: str = "", reason: str | None = None + ) -> dict[str, Any]: return { - "handover_id": "", + "handover_id": handover_id, "status": "refused", - "error_code": refusal.code, - "message": None, + "error_code": code, + "message": reason, "commit": None, "repository": str(message.get("repository") or ""), "release": str(message.get("release") or ""), "build": str(message.get("build") or ""), "attempts": 0, } + + try: + hand_over = parse_release_change(message) + except RequestRefusedError as refusal: + return refused(refusal.code) + # Who may change a release is decided here, when the change is handed + # over by its caller; a refused change is never queued. + reason = self.release_change_policy(hand_over.change, self._caller()) + if reason is not None: + return refused("NOT_AUTHORIZED", hand_over.handover_id, str(reason)) return self.queue.enqueue_job( handover_id=hand_over.handover_id, kind=RELEASE_CHANGE, @@ -238,8 +253,8 @@ def _release_change(self, message: JsonObject) -> dict[str, Any]: "message": None, "commit": None, "repository": hand_over.repository, - "release": hand_over.release, - "build": hand_over.build, + "release": hand_over.change.release, + "build": hand_over.change.build, "attempts": 0, }, now=self.clock(), diff --git a/src/microcosm_emitter/huggingface/service/queue.py b/src/microcosm_emitter/huggingface/service/queue.py index ce79b79e..3ecf7ac3 100644 --- a/src/microcosm_emitter/huggingface/service/queue.py +++ b/src/microcosm_emitter/huggingface/service/queue.py @@ -61,6 +61,15 @@ def sha256_bytes(content: bytes) -> str: return hashlib.sha256(content).hexdigest() +def sha256_file(path: Path) -> str: + """The SHA-256 of a preserved file, read in blocks.""" + digest = hashlib.sha256() + with path.open("rb") as stream: + for block in iter(lambda: stream.read(1024 * 1024), b""): + digest.update(block) + return digest.hexdigest() + + def _entry_text(entry: Mapping[str, Any]) -> str: """Order-preserving JSON, so the merged index keeps the build's key order.""" return json.dumps(dict(entry), separators=(",", ":"), allow_nan=False) diff --git a/src/microcosm_emitter/huggingface/service/releases.py b/src/microcosm_emitter/huggingface/service/releases.py index 8b913c7b..2a297217 100644 --- a/src/microcosm_emitter/huggingface/service/releases.py +++ b/src/microcosm_emitter/huggingface/service/releases.py @@ -17,22 +17,32 @@ and never touches existing tags or files outside the new build folder. Who may make a change is not settled; ``ReleaseChangePolicy`` is where that -decision plugs in, and the default allows every change. +decision plugs in. The module applies it when a change is handed over, with +the caller's identity, and the default allows every change. """ from __future__ import annotations -import hashlib -from collections.abc import Callable, Mapping +from collections.abc import Callable from dataclasses import dataclass -from pathlib import Path -from typing import Any +from typing import Literal from microcosm_emitter.huggingface.service.hub import ( + MAX_PARENT_CONFLICT_ATTEMPTS, + PRESERVED_BYTES_CHANGED, + PRESERVED_FILES_MISSING, + REPOSITORY_UNAVAILABLE, + UPLOAD_FAILED, DatasetRepository, add_operation, is_parent_conflict, ) +from microcosm_emitter.huggingface.service.queue import sha256_file +from microcosm_emitter_interface.publication import ( + PromoteBuild, + PublishRelease, + ReleaseChange, +) from microcosm_emitter_interface.releases import ( RELEASE_MANIFEST_NAME, RELEASES_RECORD_PATH, @@ -47,15 +57,32 @@ withdraw_build, ) -MAX_PARENT_CONFLICT_ATTEMPTS = 5 +PARENT_CONFLICT = "PARENT_CONFLICT" +type ReleaseStatus = Literal["pending", "published", "refused"] + + +@dataclass(frozen=True) +class ReleaseCaller: + """Who handed a release change over, as far as the emitter can tell. + + ``user`` is the account the emitter runs as, which is the build's; + ``parent_pid`` is the build process that started this emitter host. Who + may promote or withdraw is not settled, so this may gain fields once it + is. + """ -#: Returns ``None`` to allow a change, or a reason to refuse it. It receives -#: the change's request (``command``, ``repository``, ``release``, ``build``, -#: ...). Not settled yet: who may promote or withdraw. -type ReleaseChangePolicy = Callable[[Mapping[str, Any]], str | None] + user: str + parent_pid: int -def allow_every_change(change: Mapping[str, Any]) -> str | None: +#: Returns ``None`` to allow a change, or a reason to refuse it +#: (``NOT_AUTHORIZED``). It runs when the change is handed over, so a refused +#: change is never queued. +type ReleaseChangePolicy = Callable[[ReleaseChange, ReleaseCaller], str | None] + + +def allow_every_change(change: ReleaseChange, caller: ReleaseCaller) -> str | None: + """The default until who may change releases is decided.""" return None @@ -63,7 +90,7 @@ def allow_every_change(change: Mapping[str, Any]) -> str | None: class ReleaseOutcome: """One attempt's result: final unless ``retry``.""" - status: str + status: ReleaseStatus error_code: str | None = None message: str | None = None commit: str | None = None @@ -79,40 +106,30 @@ def _pending(code: str) -> ReleaseOutcome: return ReleaseOutcome("pending", code, retry=True) -def _sha256_file(path: Path) -> str: - digest = hashlib.sha256() - with path.open("rb") as stream: - for block in iter(lambda: stream.read(1024 * 1024), b""): - digest.update(block) - return digest.hexdigest() - - -def _preserved_files(request: Mapping[str, Any]) -> ReleaseOutcome | None: - for entry in request.get("files", []): - path = Path(entry["local_path"]) +def _preserved_files(change: PublishRelease) -> ReleaseOutcome | None: + for item in change.files: try: - if not path.is_file(): - return _pending("PRESERVED_FILES_MISSING") + if not item.local_path.is_file(): + return _pending(PRESERVED_FILES_MISSING) if ( - path.stat().st_size != entry["bytes"] - or _sha256_file(path) != entry["sha256"] + item.local_path.stat().st_size != item.bytes + or sha256_file(item.local_path) != item.sha256 ): return _refused( - "PRESERVED_BYTES_CHANGED", - f"{entry['name']} changed after it was handed over.", + PRESERVED_BYTES_CHANGED, + f"{item.name} changed after it was handed over.", ) except OSError: - return _pending("PRESERVED_FILES_MISSING") + return _pending(PRESERVED_FILES_MISSING) return None -def _commit_message(request: Mapping[str, Any]) -> str: - release, build = request["release"], request["build"] - command = request["command"] - if command == "publish_release": - suffix = " and make it current" if request["make_current"] else "" +def _commit_message(change: ReleaseChange) -> str: + release, build = change.release, change.build + if isinstance(change, PublishRelease): + suffix = " and make it current" if change.make_current else "" return f"Publish build {build} of {release}{suffix}" - if command == "promote_build": + if isinstance(change, PromoteBuild): return f"Make build {build} the current build of {release}" return f"Withdraw build {build} of {release}" @@ -123,26 +140,18 @@ def _first_violation(error: ReleaseRecordError) -> ReleaseOutcome: def apply_release_change( - repository: DatasetRepository, - request: Mapping[str, Any], - *, - published_at: str, - policy: ReleaseChangePolicy = allow_every_change, + repository: DatasetRepository, change: ReleaseChange, *, published_at: str ) -> ReleaseOutcome: - """Apply one release change, or report why it did not land.""" - refusal = policy(request) - if refusal is not None: - return _refused("NOT_AUTHORIZED", refusal) - publishing = request["command"] == "publish_release" - if publishing: - preserved = _preserved_files(request) + """Apply one validated release change, or report why it did not land.""" + if isinstance(change, PublishRelease): + preserved = _preserved_files(change) if preserved is not None: return preserved - release, build = request["release"], request["build"] + release, build = change.release, change.build folder = build_folder(release, build) new_files = { - f"{folder}/{entry['name']}": Path(entry["local_path"]) - for entry in (request["files"] if publishing else []) + f"{folder}/{item.name}": item.local_path + for item in (change.files if isinstance(change, PublishRelease) else ()) } for _ in range(MAX_PARENT_CONFLICT_ATTEMPTS): try: @@ -158,21 +167,21 @@ def apply_release_change( except ReleaseRecordError as error: return _first_violation(error) except Exception: - return _pending("REPOSITORY_UNAVAILABLE") + return _pending(REPOSITORY_UNAVAILABLE) record = before or ReleasesRecord.empty() try: - if publishing: + if isinstance(change, PublishRelease): if any(path.startswith(folder + "/") for path in files): - return _already_published(repository, request, record, head) + return _already_published(repository, change, record, head) after = add_build( record, release=release, build=build, - release_role=request["release_role"], + release_role=change.release_role, published_at=published_at, - make_current=request["make_current"], + make_current=change.make_current, ) - elif request["command"] == "promote_build": + elif isinstance(change, PromoteBuild): after = make_current(record, release=release, build=build) else: after = withdraw_build(record, release=release, build=build) @@ -199,7 +208,7 @@ def read_file(path: str, revision: str = head) -> bytes: *check_repository(after, [*files, *added_paths], read_file), ] except Exception: - return _pending("REPOSITORY_UNAVAILABLE") + return _pending(REPOSITORY_UNAVAILABLE) if violations: return _refused( violations[0].code, "; ".join(item.message for item in violations) @@ -208,19 +217,19 @@ def read_file(path: str, revision: str = head) -> bytes: operations.append(add_operation(RELEASES_RECORD_PATH, after.serialize())) try: commit = repository.commit( - operations, message=_commit_message(request), parent_commit=head + operations, message=_commit_message(change), parent_commit=head ) except Exception as error: if is_parent_conflict(error): continue - return _pending("UPLOAD_FAILED") + return _pending(UPLOAD_FAILED) return ReleaseOutcome("published", commit=commit, committed=True) - return _pending("PARENT_CONFLICT") + return _pending(PARENT_CONFLICT) def _already_published( repository: DatasetRepository, - request: Mapping[str, Any], + change: PublishRelease, record: ReleasesRecord, head: str, ) -> ReleaseOutcome: @@ -232,13 +241,11 @@ def _already_published( been made current since. Publishing never promotes an already published build; ``promote_build`` does that. """ - release, build = request["release"], request["build"] + release, build = change.release, change.build entry = record.releases.get(release) manifest_path = f"{build_folder(release, build)}/{RELEASE_MANIFEST_NAME}" local_manifest = next( - Path(item["local_path"]) - for item in request["files"] - if item["name"] == RELEASE_MANIFEST_NAME + item.local_path for item in change.files if item.name == RELEASE_MANIFEST_NAME ) exists_message = f"{build_folder(release, build)}/ already exists." if entry is None or build not in entry.builds: @@ -249,11 +256,11 @@ def _already_published( ) commit = repository.last_commit(manifest_path) if same else None except Exception: - return _pending("REPOSITORY_UNAVAILABLE") + return _pending(REPOSITORY_UNAVAILABLE) if not same: return _refused("BUILD_FOLDER_EXISTS", exists_message) message = None - if request["make_current"] and entry.current != build: + if change.make_current and entry.current != build: message = ( f"Build {build} of {release} was already published; the current " f"build is {entry.current}." diff --git a/src/microcosm_emitter/huggingface/service/requests.py b/src/microcosm_emitter/huggingface/service/requests.py index 97caa486..0c106b94 100644 --- a/src/microcosm_emitter/huggingface/service/requests.py +++ b/src/microcosm_emitter/huggingface/service/requests.py @@ -29,6 +29,7 @@ RELEASE_CHANGE, PromoteBuild, PublishRelease, + ReleaseChange, ReleaseFile, WithdrawBuild, ) @@ -252,14 +253,21 @@ def parse_stage_dataset_bundle(message: Mapping[str, Any]) -> BundleHandOver: @dataclass(frozen=True) class ReleaseHandOver: handover_id: str - repository: str - release: str - build: str + #: The validated change, and its canonical message, which is what is queued. + change: ReleaseChange request: dict[str, Any] + @property + def repository(self) -> str: + return self.change.repository -def parse_release_change(message: Mapping[str, Any]) -> ReleaseHandOver: - """Check a publication, promotion or withdrawal with the interface's rules.""" + +def release_change_from_message(message: Mapping[str, Any]) -> ReleaseChange: + """The typed change a message describes; ``RequestRefusedError`` if invalid. + + Only the message's shape is checked here, so the worker can rebuild the + typed change from the queued message. + """ require_supported_version(message) command = message.get("command") try: @@ -269,7 +277,7 @@ def parse_release_change(message: Mapping[str, Any]) -> ReleaseHandOver: not isinstance(entry, Mapping) for entry in entries ): raise RequestRefusedError("INVALID_REQUEST") - request = PublishRelease( + return PublishRelease( repository=message.get("repository"), release=message.get("release"), build=message.get("build"), @@ -287,36 +295,42 @@ def parse_release_change(message: Mapping[str, Any]) -> ReleaseHandOver: for entry in entries ), ) - elif command in {"promote_build", "withdraw_build"}: + if command in {"promote_build", "withdraw_build"}: change = PromoteBuild if command == "promote_build" else WithdrawBuild - request = change( + return change( repository=message.get("repository"), release=message.get("release"), build=message.get("build"), request_id=message.get("request_id"), ) - else: - raise RequestRefusedError("INVALID_REQUEST") - except (ValueError, TypeError) as error: - if isinstance(error, RequestRefusedError): - raise + except RequestRefusedError: + raise + except (ValueError, TypeError): raise RequestRefusedError("INVALID_REQUEST") from None - if command == "publish_release": - for entry, item in zip(message["files"], request.files, strict=True): - path = Path(str(entry.get("local_path"))) - if not path.is_absolute(): + raise RequestRefusedError("INVALID_REQUEST") + + +def parse_release_change(message: Mapping[str, Any]) -> ReleaseHandOver: + """Check a publication, promotion or withdrawal with the interface's rules. + + The queued request is the validated change's own message, so keys the + rules do not know never reach the queue or the hand-over identifier. + """ + change = release_change_from_message(message) + if isinstance(change, PublishRelease): + for item in change.files: + if not item.local_path.is_absolute(): raise RequestRefusedError("INVALID_REQUEST") try: - if not path.is_file(): + if not item.local_path.is_file(): raise RequestRefusedError("LOCAL_FILE_MISSING") - if path.stat().st_size != item.bytes: + if item.local_path.stat().st_size != item.bytes: raise RequestRefusedError("FILE_SIZE_DIFFERS") except OSError: raise RequestRefusedError("LOCAL_FILE_MISSING") from None + canonical = change.message() return ReleaseHandOver( - handover_id=handover_identifier(RELEASE_CHANGE, dict(message)), - repository=request.repository, - release=request.release, - build=request.build, - request=dict(message), + handover_id=handover_identifier(RELEASE_CHANGE, canonical), + change=change, + request=canonical, ) diff --git a/src/microcosm_emitter/huggingface/service/run_files.py b/src/microcosm_emitter/huggingface/service/run_files.py index 3204a97b..73a92249 100644 --- a/src/microcosm_emitter/huggingface/service/run_files.py +++ b/src/microcosm_emitter/huggingface/service/run_files.py @@ -16,6 +16,7 @@ from typing import Any from microcosm_emitter.huggingface.service.hub import ( + MAX_PARENT_CONFLICT_ATTEMPTS, DatasetRepository, add_operation, is_absent, @@ -26,7 +27,6 @@ #: Staging contract version 1's index of runs, at the repository root. RUNS_INDEX_PATH = "runs.json" RUNS_INDEX_SCHEMA_VERSION = 1 -MAX_PARENT_CONFLICT_ATTEMPTS = 5 def _jsonable(value: Any) -> Any: diff --git a/src/microcosm_emitter/telemetry/service/collector.py b/src/microcosm_emitter/telemetry/service/collector.py index e9c4ba8c..f6ecd9d6 100644 --- a/src/microcosm_emitter/telemetry/service/collector.py +++ b/src/microcosm_emitter/telemetry/service/collector.py @@ -13,7 +13,7 @@ _collector_origin, _development_collector_url, _http_post, - _huggingface_token, + huggingface_token, ) from microcosm_emitter.core.constants import ( INITIAL_RETRY_SECONDS, @@ -66,7 +66,7 @@ def __init__( self.session = session or CollectorSession( self.collector_url, post=lambda *args: _http_post(*args), - credential=lambda: _huggingface_token(), + credential=lambda: huggingface_token(), ) self._registered: set[str] = set() self._warned_no_token = False diff --git a/tests/huggingface/test_release_writer.py b/tests/huggingface/test_release_writer.py index 36784d8a..ab239999 100644 --- a/tests/huggingface/test_release_writer.py +++ b/tests/huggingface/test_release_writer.py @@ -10,10 +10,7 @@ from microcosm_emitter.host.contracts import ModuleContext from microcosm_emitter.huggingface.service.hub import DatasetRepository from microcosm_emitter.huggingface.service.module import HuggingFaceModule -from microcosm_emitter.huggingface.service.releases import ( - allow_every_change, - apply_release_change, -) +from microcosm_emitter.huggingface.service.releases import apply_release_change from microcosm_emitter_interface.publication import ( PromoteBuild, PublishRelease, @@ -69,12 +66,9 @@ def publish_request(tmp_path, build, *, make_current=False, **options): ) -def apply(hub, request, *, published_at="2026-10-20T00:00:00+00:00", policy=None): +def apply(hub, request, *, published_at="2026-10-20T00:00:00+00:00"): return apply_release_change( - DatasetRepository(PRIVATE_REPOSITORY, hub), - request.message(), - published_at=published_at, - policy=policy or allow_every_change, + DatasetRepository(PRIVATE_REPOSITORY, hub), request, published_at=published_at ) @@ -297,21 +291,32 @@ def test_local_bytes_changed_after_the_hand_over_are_refused(hub, tmp_path): assert hub.commits(PRIVATE_REPOSITORY) == [] -def test_a_change_policy_can_refuse_a_promotion(hub, tmp_path): - request, _ = publish_request(tmp_path, FIRST_BUILD) - assert apply(hub, request).status == "published" - - def only_listing(change): - return None if change["command"] == "publish_release" else "not permitted" +def test_a_change_policy_can_refuse_a_promotion(tmp_path, hub): + def only_listing(change, caller): + return None if isinstance(change, PublishRelease) else "not permitted" - outcome = apply( - hub, - PromoteBuild( - repository=PRIVATE_REPOSITORY, release=NATIONAL, build=FIRST_BUILD - ), - policy=only_listing, + module = HuggingFaceModule( + {"handover_database_path": str(tmp_path / "events.handovers.sqlite3")}, + ModuleContext(os.getpid()), + api_factory=lambda: hub, + release_change_policy=only_listing, + background_workers=False, + ) + module.initialize() + try: + request, _ = publish_request(tmp_path, FIRST_BUILD) + assert module.query(request.message())["receipt"]["status"] == "pending" + promotion_receipt = module.query( + PromoteBuild( + repository=PRIVATE_REPOSITORY, release=NATIONAL, build=FIRST_BUILD + ).message() + )["receipt"] + finally: + module.close(0) + assert (promotion_receipt["status"], promotion_receipt["error_code"]) == ( + "refused", + "NOT_AUTHORIZED", ) - assert (outcome.status, outcome.error_code) == ("refused", "NOT_AUTHORIZED") def test_existing_tags_old_files_and_staged_bundles_are_untouched(hub, tmp_path): @@ -380,29 +385,23 @@ def test_module_refuses_a_malformed_release_change_without_queueing_it( assert controls.hub.commits(PRIVATE_REPOSITORY) == [] -def test_the_module_writer_uses_the_injected_policy(tmp_path, hub): +def test_a_change_the_policy_refuses_is_never_written(tmp_path, hub): module = HuggingFaceModule( {"handover_database_path": str(tmp_path / "events.handovers.sqlite3")}, ModuleContext(os.getpid()), api_factory=lambda: hub, - release_change_policy=lambda change: "refused by policy", + release_change_policy=lambda change, caller: "refused by policy", background_workers=False, ) module.initialize() try: request, _ = publish_request(tmp_path, FIRST_BUILD) - module.query(request.message()) + receipt = module.query(request.message())["receipt"] module.delivery.run_cycle() - receipt = module.query( - { - "protocol_version": 1, - "command": "handover_receipt", - "handover_id": request.handover_id(), - } - )["receipt"] finally: module.close(0) assert (receipt["status"], receipt["error_code"]) == ("refused", "NOT_AUTHORIZED") + assert hub.commits(PRIVATE_REPOSITORY) == [] def queue_change(controls, request): @@ -654,3 +653,68 @@ def test_release_changes_skip_the_commit_spacing_but_bundles_keep_it( controls.clock.advance(1) controls.module.delivery.run_cycle() assert receipt_status(controls, bundle.handover_id()) == "uploaded" + + +def test_release_outcome_statuses_are_literal(): + import typing + + from microcosm_emitter.huggingface.service.releases import ReleaseOutcome + + status = typing.get_type_hints(ReleaseOutcome)["status"] + assert set(typing.get_args(status.__value__)) == {"pending", "published", "refused"} + + +def test_the_queued_request_is_the_validated_typed_request(hugging_face_module): + controls = hugging_face_module + request = promotion(FIRST_BUILD) + message = {**request.message(), "unvalidated": "extra"} + receipt = controls.module.query(message)["receipt"] + assert receipt["handover_id"] == request.handover_id() + job = controls.module.queue.claim_job( + PRIVATE_REPOSITORY, now=controls.clock(), lease_seconds=1 + ) + assert job.request == request.message() + + +def test_the_change_policy_receives_the_typed_change_and_its_caller(tmp_path, hub): + import getpass + + seen = [] + + def record_and_refuse(change, caller): + seen.append((change, caller)) + return "refused by policy" + + module = HuggingFaceModule( + {"handover_database_path": str(tmp_path / "events.handovers.sqlite3")}, + ModuleContext(os.getpid()), + api_factory=lambda: hub, + release_change_policy=record_and_refuse, + background_workers=False, + ) + module.initialize() + try: + request = promotion(FIRST_BUILD) + receipt = module.query(request.message())["receipt"] + assert (receipt["status"], receipt["error_code"], receipt["message"]) == ( + "refused", + "NOT_AUTHORIZED", + "refused by policy", + ) + # Refused at hand-over: nothing is queued. + assert module.queue.job_receipt(request.handover_id()) is None + finally: + module.close(0) + ((change, caller),) = seen + assert change == request + assert (caller.parent_pid, caller.user) == (os.getpid(), getpass.getuser()) + + +def test_the_default_policy_allows_every_change(hugging_face_module): + from microcosm_emitter.huggingface.service.releases import ( + ReleaseCaller, + allow_every_change, + ) + + caller = ReleaseCaller(user="fixture-user", parent_pid=1) + assert allow_every_change(promotion(FIRST_BUILD), caller) is None diff --git a/tests/telemetry/test_delivery.py b/tests/telemetry/test_delivery.py index 84986aa1..b8175dcc 100644 --- a/tests/telemetry/test_delivery.py +++ b/tests/telemetry/test_delivery.py @@ -219,7 +219,7 @@ def test_collector_delivery_exchanges_hf_token_then_flushes( queued = spool.append(registration, _event()) requests: list[tuple[str, dict[str, object], str]] = [] - monkeypatch.setattr(collector_module, "_huggingface_token", lambda: "hf-secret") + monkeypatch.setattr(collector_module, "huggingface_token", lambda: "hf-secret") def fake_post(url, payload, token, *, timeout=5.0): requests.append((url, payload, token)) @@ -254,7 +254,7 @@ def test_non_org_credential_keeps_event_local(tmp_path, monkeypatch, capsys) -> spool.register(registration) spool.append(registration, _event()) requests: list[dict[str, object]] = [] - monkeypatch.setattr(collector_module, "_huggingface_token", lambda: "hf-outsider") + monkeypatch.setattr(collector_module, "huggingface_token", lambda: "hf-outsider") def reject(url, payload, token, *, timeout=5.0): requests.append(payload) @@ -281,7 +281,7 @@ def test_identity_provider_outage_keeps_events_eligible_for_retry( registration = _registration() spool.register(registration) spool.append(registration, _event()) - monkeypatch.setattr(collector_module, "_huggingface_token", lambda: "hf-member") + monkeypatch.setattr(collector_module, "huggingface_token", lambda: "hf-member") monkeypatch.setattr( collector_module, "_http_post", @@ -300,7 +300,7 @@ def test_missing_credential_never_contacts_collector_and_stays_local_only( registration = _registration() spool.register(registration) spool.append(registration, _event()) - monkeypatch.setattr(collector_module, "_huggingface_token", lambda: None) + monkeypatch.setattr(collector_module, "huggingface_token", lambda: None) monkeypatch.setattr( collector_module, "_http_post", @@ -314,7 +314,7 @@ def test_missing_credential_never_contacts_collector_and_stays_local_only( assert spool.pending_runs() == [] reopened = EventSpool(spool_path) - monkeypatch.setattr(collector_module, "_huggingface_token", lambda: "hf-later") + monkeypatch.setattr(collector_module, "huggingface_token", lambda: "hf-later") assert reopened.pending_runs() == [] @@ -335,9 +335,7 @@ def test_permanent_collector_rejection_retains_only_rejected_producer( rejected = spool.append(bad, _event()) spool.append(good, _event()) requests = [] - monkeypatch.setattr( - collector_module, "_huggingface_token", lambda: "hf_test_member" - ) + monkeypatch.setattr(collector_module, "huggingface_token", lambda: "hf_test_member") def post(url, payload, token): requests.append((url, payload)) @@ -399,9 +397,7 @@ def test_transient_collector_rejection_remains_retryable( clock = [100.0] attempts = [] monkeypatch.setattr(collector_module.time, "monotonic", lambda: clock[0]) - monkeypatch.setattr( - collector_module, "_huggingface_token", lambda: "hf_test_member" - ) + monkeypatch.setattr(collector_module, "huggingface_token", lambda: "hf_test_member") def post(url, payload, token): if url.endswith("/exchange"): diff --git a/tests/test_boundaries.py b/tests/test_boundaries.py index 785b9365..72c1a846 100644 --- a/tests/test_boundaries.py +++ b/tests/test_boundaries.py @@ -159,3 +159,50 @@ def audit(event, args): assert "microcosm_emitter" not in sys.modules """ subprocess.run([sys.executable, "-c", script], check=True) + + +def module_level_names(path): + for node in ast.parse(path.read_text()).body: + if isinstance(node, ast.Assign): + for target in node.targets: + if isinstance(target, ast.Name): + yield target.id + elif isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name): + yield node.target.id + + +def test_hand_over_codes_and_limits_are_defined_once(): + shared = { + "MAX_PARENT_CONFLICT_ATTEMPTS", + "UPLOAD_FAILED", + "REPOSITORY_UNAVAILABLE", + "PRESERVED_FILES_MISSING", + "PRESERVED_BYTES_CHANGED", + "RELEASE_CHANGE", + } + paths = [ + *(ROOT / "src/microcosm_emitter").rglob("*.py"), + *(INTERFACE_SOURCE / "microcosm_emitter_interface").rglob("*.py"), + ] + defined_in = {name: [] for name in shared} + for path in paths: + for name in module_level_names(path): + if name in shared: + defined_in[name].append(path.relative_to(ROOT)) + assert {name: len(places) for name, places in defined_in.items()} == { + name: 1 for name in shared + }, defined_in + + +def test_the_credential_lookup_is_public(): + from microcosm_emitter.core.auth import huggingface_token + + assert callable(huggingface_token) + + +def test_no_module_imports_the_private_credential_lookup(): + for path in (ROOT / "src/microcosm_emitter").rglob("*.py"): + for node in ast.walk(ast.parse(path.read_text())): + if isinstance(node, ast.ImportFrom): + names = {alias.name for alias in node.names} + assert "_huggingface_token" not in names, path From e8d8ecc140a68fbda57f075f7157fba465508a5e Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 20 Oct 2026 21:32:10 +0400 Subject: [PATCH 09/24] Test that a queued release change never undoes a newer one A release change that failed for any reason other than a broken rule stayed pending, was retried without limit and in backoff order, and was applied to whatever record it found. So an older promotion could land after a newer change from this or another machine and put back an older current build, and a change pending for days could land after a login. The tests cover the #24 review's case (another machine's newer change lands first), the same case with the newer change made on this machine and changing nothing, a change pending for two days before and after a login, and two guards: changes handed over in order all apply, and a plain publication queued before a newer change still lands. Red: 4 failed, 34 passed. The older SECOND build became current in both ordering cases ("assert 'uk-frs-calib...323Z-6cba5d71' == 'uk-frs-calib...443Z-d12ff9b3'"); the change pending for two days was "published"; the expired change stayed ("pending", "CREDENTIAL_MISSING"). Co-Authored-By: Claude Opus 5.5 --- tests/huggingface/test_release_writer.py | 160 +++++++++++++++++++++++ 1 file changed, 160 insertions(+) diff --git a/tests/huggingface/test_release_writer.py b/tests/huggingface/test_release_writer.py index ab239999..15762257 100644 --- a/tests/huggingface/test_release_writer.py +++ b/tests/huggingface/test_release_writer.py @@ -718,3 +718,163 @@ def test_the_default_policy_allows_every_change(hugging_face_module): caller = ReleaseCaller(user="fixture-user", parent_pid=1) assert allow_every_change(promotion(FIRST_BUILD), caller) is None + + +THIRD_BUILD = "uk-frs-calibration-attempt-20261020T101500Z-0a1b2c3d" +TWO_DAYS_SECONDS = 2 * 24 * 3600 + + +def hand_over(controls, request): + return controls.module.query(request.message())["receipt"] + + +def receipt_of(controls, request): + return controls.module.query( + { + "protocol_version": 1, + "command": "handover_receipt", + "handover_id": request.handover_id(), + } + )["receipt"] + + +def status_of(receipt): + return receipt["status"], receipt["error_code"] + + +def queue_a_promotion_that_fails_once(controls, build): + """Hand over a promotion whose first commit fails; it stays pending.""" + controls.hub.fail_next("create_commit", FakeHubHTTPError(503)) + request = promotion(build) + hand_over(controls, request) + controls.module.delivery.run_cycle() + assert status_of(receipt_of(controls, request)) == ("pending", "UPLOAD_FAILED") + return request + + +def test_a_promotion_queued_before_another_machine_changed_current_is_refused( + hugging_face_module, +): + """The #24 review's case: another machine's later change landed first.""" + controls = hugging_face_module + for build in (FIRST_BUILD, SECOND_BUILD): + request, _ = publish_request(controls.tmp_path, build) + assert queue_change(controls, request)["status"] == "published" + queued = queue_a_promotion_that_fails_once(controls, SECOND_BUILD) + controls.clock.advance(1) + other_machine = apply_release_change( + DatasetRepository(PRIVATE_REPOSITORY, controls.hub), + promotion(FIRST_BUILD), + published_at="2026-10-20T00:00:00+00:00", + ) + assert other_machine.status == "published" + assert current_of(controls).current == FIRST_BUILD + + controls.clock.advance(600) + controls.module.delivery.run_cycle() + + assert current_of(controls).current == FIRST_BUILD + assert status_of(receipt_of(controls, queued)) == ( + "refused", + "NEWER_CHANGE_LANDED", + ) + + +def test_an_older_promotion_never_lands_after_a_newer_one_from_this_machine( + hugging_face_module, +): + """The newer promotion changes nothing (its build is already current).""" + controls = hugging_face_module + publish_both(controls) + older = queue_a_promotion_that_fails_once(controls, SECOND_BUILD) + controls.clock.advance(1) + assert queue_change(controls, promotion(FIRST_BUILD))["status"] == "published" + + controls.clock.advance(600) + controls.module.delivery.run_cycle() + + assert current_of(controls).current == FIRST_BUILD + assert status_of(receipt_of(controls, older)) == ( + "refused", + "NEWER_CHANGE_LANDED", + ) + + +def test_a_change_pending_for_days_is_refused_after_a_login_not_applied( + hugging_face_module, +): + controls = hugging_face_module + publish_both(controls) + controls.remove_credential() + request = promotion(SECOND_BUILD) + hand_over(controls, request) + controls.module.delivery.run_cycle() + assert status_of(receipt_of(controls, request)) == ( + "pending", + "CREDENTIAL_MISSING", + ) + + controls.clock.advance(TWO_DAYS_SECONDS) + controls.restore_credential() + controls.module.delivery.run_cycle() + + assert status_of(receipt_of(controls, request)) == ("refused", "REQUEST_EXPIRED") + assert current_of(controls).current == FIRST_BUILD + + +def test_a_change_that_cannot_land_becomes_final_once_it_expires( + hugging_face_module, +): + controls = hugging_face_module + publish_both(controls) + controls.remove_credential() + request = promotion(SECOND_BUILD) + hand_over(controls, request) + controls.module.delivery.run_cycle() + + controls.clock.advance(TWO_DAYS_SECONDS) + controls.module.delivery.run_cycle() + + assert status_of(receipt_of(controls, request)) == ("refused", "REQUEST_EXPIRED") + + +def test_changes_handed_over_in_order_on_this_machine_all_apply( + hugging_face_module, +): + controls = hugging_face_module + publish_both(controls) + first = promotion(SECOND_BUILD) + hand_over(controls, first) + controls.clock.advance(1) + second = promotion(FIRST_BUILD) + hand_over(controls, second) + + for _ in range(2): + controls.module.delivery.run_cycle() + controls.clock.advance(1) + + assert status_of(receipt_of(controls, first)) == ("published", None) + assert status_of(receipt_of(controls, second)) == ("published", None) + assert current_of(controls).current == FIRST_BUILD + + +def test_a_publication_queued_before_a_newer_change_still_lands( + hugging_face_module, +): + """Adding a build undoes no newer change, and two publications must both + land (plan item 5), so a plain publication is never refused for order.""" + controls = hugging_face_module + publish_both(controls) + controls.hub.fail_next("create_commit", FakeHubHTTPError(503)) + third, _ = publish_request(controls.tmp_path, THIRD_BUILD) + hand_over(controls, third) + controls.module.delivery.run_cycle() + controls.clock.advance(1) + assert queue_change(controls, promotion(SECOND_BUILD))["status"] == "published" + + controls.clock.advance(600) + controls.module.delivery.run_cycle() + + assert status_of(receipt_of(controls, third)) == ("published", None) + assert THIRD_BUILD in current_of(controls).builds + assert current_of(controls).current == SECOND_BUILD From f05f813befd43dd2e7b6d003707072ded5425736 Mon Sep 17 00:00:00 2001 From: Anthony Volk <14987227+anth-volk@users.noreply.github.com> Date: Tue, 20 Oct 2026 21:35:55 +0400 Subject: [PATCH 10/24] Expire queued release changes and refuse ones a newer change overtook The mechanism, also described in releases.py and the README: - Expiry: a release change still pending an hour after its hand-over (RELEASE_CHANGE_EXPIRY_SECONDS) becomes refused with REQUEST_EXPIRED, also when no credential is present, so it never lands after a later login. - Order: an ordered change (a promotion, a withdrawal, or a publication that makes its build current) is refused with NEWER_CHANGE_LANDED when an ordered change requested after it has already landed on the repository's record. The writer learns that from this machine's queue, which also holds later changes that left the record unchanged, and from the title of the commit that last changed releases.json. Every writer commit's title now ends "[newest request