From 1577ce6c0634b1a89f295531ec01c5c8bc017efc Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 13 Jul 2026 15:38:50 +0200 Subject: [PATCH] source store: drop the cvmfs freshness probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Boot already trusts read-only stores as published, so the CVMFS revision token had no boot-time role, and the watcher — its only would-be consumer — accepts any probe. Remove cvmfs_revision_token, the factory's 'freshness: cvmfs' branch, and the freshness_path config surface that existed only to serve it. --- lib/galaxy/tools/source_store/factory.py | 27 +------- lib/galaxy/tools/source_store/freshness.py | 65 ++++--------------- lib/galaxy/tools/source_store/watcher.py | 3 +- .../app/tools/source_store/test_freshness.py | 33 ++-------- 4 files changed, 20 insertions(+), 108 deletions(-) diff --git a/lib/galaxy/tools/source_store/factory.py b/lib/galaxy/tools/source_store/factory.py index 411b906ea9f..f2043577039 100644 --- a/lib/galaxy/tools/source_store/factory.py +++ b/lib/galaxy/tools/source_store/factory.py @@ -3,12 +3,9 @@ import logging from typing import TYPE_CHECKING -from sqlalchemy.engine import make_url - from galaxy.tool_util.toolbox.parser import get_toolbox_parser from .composite import CompositeToolSourceStore from .freshness import ( - cvmfs_probe, FreshnessProbe, tool_confs_probe, ) @@ -39,19 +36,6 @@ def _build_default_store( return SqlAlchemyToolSourceStore(url=url, read_only=False, freshness_probe=tool_confs_probe(config)) -def _sqlite_database_path(url: str) -> str | None: - """On-disk file behind a sqlite URL, or None for other backends.""" - parsed = make_url(url) - if parsed.drivername.split("+")[0] != "sqlite": - return None - database = parsed.database - if not database or database == ":memory:": - return None - if database.startswith("file:"): - database = database[len("file:") :].split("?", 1)[0] - return database or None - - def _build_freshness_probe( name: str, spec: dict, @@ -62,15 +46,8 @@ def _build_freshness_probe( return None if freshness == "tool_confs": return tool_confs_probe(config) - if freshness == "cvmfs": - probe_path = spec.get("freshness_path") or _sqlite_database_path(spec["url"]) - if not probe_path: - raise ConfigurationError( - f"tool_source_stores[{name!r}] uses freshness: cvmfs with a non-file url; set freshness_path" - ) - return cvmfs_probe(probe_path) raise ConfigurationError( - f"tool_source_stores[{name!r}] freshness must be 'cvmfs', 'tool_confs', or 'none' (got {freshness!r})" + f"tool_source_stores[{name!r}] freshness must be 'tool_confs' or 'none' (got {freshness!r})" ) @@ -82,7 +59,7 @@ def build_named_store( """Build a single named store from a ``tool_source_stores`` entry. ``spec`` is the dict from galaxy.yml - a SQLAlchemy ``url`` plus - optional ``read_only``, ``freshness``, and ``freshness_path`` keys. + optional ``read_only`` and ``freshness`` keys. Named stores get no probe unless one is declared: a store populated on a different host (the CVMFS publishing model) would never match a locally-computed conf hash, so ``tool_confs`` cannot be the default diff --git a/lib/galaxy/tools/source_store/freshness.py b/lib/galaxy/tools/source_store/freshness.py index d80b6a0d886..62b6b6ee1e7 100644 --- a/lib/galaxy/tools/source_store/freshness.py +++ b/lib/galaxy/tools/source_store/freshness.py @@ -7,26 +7,18 @@ compares. A match certifies the store still covers the current tree, so boot skips the per-path coverage scan (and the populate it would trigger). A mismatch is always safe — it only falls back to scanning/populating. -Two probe kinds exist: +The built-in probe is ``tool_confs``: md5 over the tool and data-manager +conf file contents, plus the (recursive) directory mtimes of any +``tool_dir`` entries they declare. This captures tool +additions/removals/renames — the same class of drift the coverage scan +detects — without touching individual tool files. In-place edits to a +tool's XML are invisible to both, by design: content changes are the +incremental populate's job (raw-file md5), not the coverage check's. +Wired to the default (writable) store automatically. -- ``tool_confs``: md5 over the tool and data-manager conf file contents, - plus the (recursive) directory mtimes of any ``tool_dir`` entries they - declare. This captures tool additions/removals/renames — the same class - of drift the coverage scan detects — without touching individual tool - files. In-place edits to a tool's XML are invisible to both, by design: - content changes are the incremental populate's job (raw-file md5), not - the coverage check's. Wired to the default (writable) store - automatically. -- ``cvmfs``: the CernVM-FS repository revision, read from the - ``user.revision`` extended attribute the CVMFS client exposes on the - mount point. One syscall covers every file in the repository. This - probe only feeds the watcher's change detection: a revision transition - means a new publish landed and cached state should reload. It is *not* - a boot-time token comparison — read-only stores are trusted as - published (see ``SqlAlchemyToolSourceStore.index_is_fresh``: the bundle - ships in the same transaction as the tools it indexes), and a stamped - revision could never match anyway, since the publisher populates inside - the transaction at revision N while clients see N+1. +Read-only stores need no probe for boot freshness — they are trusted as +published (see ``SqlAlchemyToolSourceStore.index_is_fresh``); a probe on +such a store only feeds the watcher's change detection. """ import hashlib @@ -104,38 +96,3 @@ def tool_confs_token(config: "GalaxyAppConfiguration") -> str: def tool_confs_probe(config: "GalaxyAppConfiguration") -> FreshnessProbe: return lambda: tool_confs_token(config) - - -def _os_getxattr(path: str, attribute: str) -> bytes: - # ``os.getxattr`` only exists on Linux; CVMFS deployments are Linux. - getxattr = getattr(os, "getxattr", None) - if getxattr is None: - raise FreshnessProbeError("extended attributes are not supported on this platform") - return getxattr(path, attribute) - - -def cvmfs_revision_token(path: str, _getxattr: Callable[[str, str], bytes] = _os_getxattr) -> str: - """CVMFS repository revision token for the repository containing ``path``. - - The CVMFS client exposes repository metadata as extended attributes on - the mount point, so ascend from ``path`` until ``user.revision`` - answers. Raises :class:`FreshnessProbeError` when no ancestor exposes - it — ``path`` isn't on CVMFS, or the repository isn't mounted (in - which case its tools are unreadable anyway, and "not fresh" is the - right verdict). - """ - probe_path = os.path.abspath(path) - while True: - try: - revision = _getxattr(probe_path, "user.revision") - except OSError: - parent = os.path.dirname(probe_path) - if parent == probe_path: - raise FreshnessProbeError(f"no CVMFS revision xattr found on any ancestor of {path}") - probe_path = parent - continue - return f"cvmfs:{os.path.basename(probe_path)}:{revision.decode()}" - - -def cvmfs_probe(path: str) -> FreshnessProbe: - return lambda: cvmfs_revision_token(path) diff --git a/lib/galaxy/tools/source_store/watcher.py b/lib/galaxy/tools/source_store/watcher.py index b92373f9070..dc1405c1b52 100644 --- a/lib/galaxy/tools/source_store/watcher.py +++ b/lib/galaxy/tools/source_store/watcher.py @@ -4,8 +4,7 @@ A store published outside this Galaxy process — the CVMFS model, where a publisher repopulates the sqlite bundle in the same transaction that ships new tools — changes without any local filesystem event (inotify does not fire on CVMFS). Polling each store's freshness probe is the only reliable -signal, and it is cheap: one probe per store per tick, a single -extended-attribute read for CVMFS stores. +signal, and it is cheap: one probe per store per tick. The watcher itself is deliberately dumb: it detects token transitions and hands the changed store names to ``on_change``. Reload mechanics — diff --git a/test/unit/app/tools/source_store/test_freshness.py b/test/unit/app/tools/source_store/test_freshness.py index c66f8879510..d991ddc4aca 100644 --- a/test/unit/app/tools/source_store/test_freshness.py +++ b/test/unit/app/tools/source_store/test_freshness.py @@ -2,11 +2,8 @@ import logging -import pytest - from galaxy.tools.source_store.composite import CompositeToolSourceStore from galaxy.tools.source_store.freshness import ( - cvmfs_revision_token, FreshnessProbeError, tool_confs_token, ) @@ -53,24 +50,6 @@ def test_tool_confs_token_sees_tool_dir_membership_changes(tmp_path): assert tool_confs_token(cfg) != after_top_level -def test_cvmfs_revision_token_ascends_to_the_mount_root(): - def fake_getxattr(path, attribute): - if path == "/cvmfs/main.galaxyproject.org" and attribute == "user.revision": - return b"1042" - raise OSError(61, "no attribute") - - token = cvmfs_revision_token("/cvmfs/main.galaxyproject.org/galaxy/store.sqlite", _getxattr=fake_getxattr) - assert token == "cvmfs:main.galaxyproject.org:1042" - - -def test_cvmfs_revision_token_raises_off_cvmfs(): - def fake_getxattr(path, attribute): - raise OSError(61, "no attribute") - - with pytest.raises(FreshnessProbeError): - cvmfs_revision_token("/plain/local/path", _getxattr=fake_getxattr) - - def test_index_is_fresh_tracks_probe(tmp_path): current = {"token": "confs:a"} store = SqlAlchemyToolSourceStore(url=f"sqlite:///{tmp_path}/a.sqlite", freshness_probe=lambda: current["token"]) @@ -92,7 +71,7 @@ def test_index_is_fresh_false_when_probe_fails(tmp_path): raise FreshnessProbeError("repo not mounted") store = SqlAlchemyToolSourceStore(url=f"sqlite:///{tmp_path}/a.sqlite", freshness_probe=broken_probe) - store.store_index(ToolIndex(freshness_token="cvmfs:r:1")) + store.store_index(ToolIndex(freshness_token="bundle:1")) assert store.index_is_fresh() is False @@ -102,14 +81,14 @@ def _stamped_store(path, token, probe_token, read_only=False): def test_composite_fresh_when_all_members_fresh(tmp_path): - ro = _stamped_store(tmp_path / "ro.sqlite", "cvmfs:r:1", "cvmfs:r:1", read_only=True) + ro = _stamped_store(tmp_path / "ro.sqlite", "bundle:1", "bundle:1", read_only=True) rw = _stamped_store(tmp_path / "rw.sqlite", "confs:x", "confs:x") composite = CompositeToolSourceStore(members=[("ro", ro), ("rw", rw)], default="rw") assert composite.index_is_fresh() is True def test_composite_stale_writable_member_wins(tmp_path): - ro = _stamped_store(tmp_path / "ro.sqlite", "cvmfs:r:1", "cvmfs:r:1", read_only=True) + ro = _stamped_store(tmp_path / "ro.sqlite", "bundle:1", "bundle:1", read_only=True) rw = _stamped_store(tmp_path / "rw.sqlite", "confs:x", "confs:y") composite = CompositeToolSourceStore(members=[("ro", ro), ("rw", rw)], default="rw") assert composite.index_is_fresh() is False @@ -118,14 +97,14 @@ def test_composite_stale_writable_member_wins(tmp_path): def test_read_only_store_trusts_schema_valid_index_over_probe(tmp_path): # Stamped token and probe value disagree, but a read-only store is # trusted whenever its index loads — the probe only feeds the watcher. - ro = _stamped_store(tmp_path / "ro.sqlite", "cvmfs:r:1", "cvmfs:r:2", read_only=True) + ro = _stamped_store(tmp_path / "ro.sqlite", "bundle:1", "bundle:2", read_only=True) assert ro.index_is_fresh() is True def test_read_only_store_without_index_is_not_fresh(tmp_path): SqlAlchemyToolSourceStore(url=f"sqlite:///{tmp_path}/ro.sqlite") ro = SqlAlchemyToolSourceStore( - url=f"sqlite:///{tmp_path}/ro.sqlite", read_only=True, freshness_probe=lambda: "cvmfs:r:1" + url=f"sqlite:///{tmp_path}/ro.sqlite", read_only=True, freshness_probe=lambda: "bundle:1" ) assert ro.index_is_fresh() is False @@ -141,7 +120,7 @@ def test_composite_read_only_member_without_index_warns_but_stays_fresh(tmp_path def test_composite_member_without_probe_downgrades_to_none(tmp_path): - ro = _stamped_store(tmp_path / "ro.sqlite", "cvmfs:r:1", "cvmfs:r:1", read_only=True) + ro = _stamped_store(tmp_path / "ro.sqlite", "bundle:1", "bundle:1", read_only=True) rw = SqlAlchemyToolSourceStore(url=f"sqlite:///{tmp_path}/rw.sqlite") composite = CompositeToolSourceStore(members=[("ro", ro), ("rw", rw)], default="rw") assert composite.index_is_fresh() is None