source store: drop the cvmfs freshness probe

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.
This commit is contained in:
mvdbeek
2026-07-13 15:38:50 +02:00
parent 7876758c12
commit 1577ce6c06
4 changed files with 20 additions and 108 deletions
+2 -25
View File
@@ -3,12 +3,9 @@
import logging import logging
from typing import TYPE_CHECKING from typing import TYPE_CHECKING
from sqlalchemy.engine import make_url
from galaxy.tool_util.toolbox.parser import get_toolbox_parser from galaxy.tool_util.toolbox.parser import get_toolbox_parser
from .composite import CompositeToolSourceStore from .composite import CompositeToolSourceStore
from .freshness import ( from .freshness import (
cvmfs_probe,
FreshnessProbe, FreshnessProbe,
tool_confs_probe, tool_confs_probe,
) )
@@ -39,19 +36,6 @@ def _build_default_store(
return SqlAlchemyToolSourceStore(url=url, read_only=False, freshness_probe=tool_confs_probe(config)) 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( def _build_freshness_probe(
name: str, name: str,
spec: dict, spec: dict,
@@ -62,15 +46,8 @@ def _build_freshness_probe(
return None return None
if freshness == "tool_confs": if freshness == "tool_confs":
return tool_confs_probe(config) 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( 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. """Build a single named store from a ``tool_source_stores`` entry.
``spec`` is the dict from galaxy.yml - a SQLAlchemy ``url`` plus ``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 Named stores get no probe unless one is declared: a store populated on
a different host (the CVMFS publishing model) would never match a a different host (the CVMFS publishing model) would never match a
locally-computed conf hash, so ``tool_confs`` cannot be the default locally-computed conf hash, so ``tool_confs`` cannot be the default
+11 -54
View File
@@ -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). 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. 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, Read-only stores need no probe for boot freshness — they are trusted as
plus the (recursive) directory mtimes of any ``tool_dir`` entries they published (see ``SqlAlchemyToolSourceStore.index_is_fresh``); a probe on
declare. This captures tool additions/removals/renames — the same class such a store only feeds the watcher's change detection.
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.
""" """
import hashlib import hashlib
@@ -104,38 +96,3 @@ def tool_confs_token(config: "GalaxyAppConfiguration") -> str:
def tool_confs_probe(config: "GalaxyAppConfiguration") -> FreshnessProbe: def tool_confs_probe(config: "GalaxyAppConfiguration") -> FreshnessProbe:
return lambda: tool_confs_token(config) 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)
+1 -2
View File
@@ -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 publisher repopulates the sqlite bundle in the same transaction that ships
new tools — changes without any local filesystem event (inotify does not new tools — changes without any local filesystem event (inotify does not
fire on CVMFS). Polling each store's freshness probe is the only reliable 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 signal, and it is cheap: one probe per store per tick.
extended-attribute read for CVMFS stores.
The watcher itself is deliberately dumb: it detects token transitions and The watcher itself is deliberately dumb: it detects token transitions and
hands the changed store names to ``on_change``. Reload mechanics — hands the changed store names to ``on_change``. Reload mechanics —
@@ -2,11 +2,8 @@
import logging import logging
import pytest
from galaxy.tools.source_store.composite import CompositeToolSourceStore from galaxy.tools.source_store.composite import CompositeToolSourceStore
from galaxy.tools.source_store.freshness import ( from galaxy.tools.source_store.freshness import (
cvmfs_revision_token,
FreshnessProbeError, FreshnessProbeError,
tool_confs_token, 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 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): def test_index_is_fresh_tracks_probe(tmp_path):
current = {"token": "confs:a"} current = {"token": "confs:a"}
store = SqlAlchemyToolSourceStore(url=f"sqlite:///{tmp_path}/a.sqlite", freshness_probe=lambda: current["token"]) 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") raise FreshnessProbeError("repo not mounted")
store = SqlAlchemyToolSourceStore(url=f"sqlite:///{tmp_path}/a.sqlite", freshness_probe=broken_probe) 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 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): 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") rw = _stamped_store(tmp_path / "rw.sqlite", "confs:x", "confs:x")
composite = CompositeToolSourceStore(members=[("ro", ro), ("rw", rw)], default="rw") composite = CompositeToolSourceStore(members=[("ro", ro), ("rw", rw)], default="rw")
assert composite.index_is_fresh() is True assert composite.index_is_fresh() is True
def test_composite_stale_writable_member_wins(tmp_path): 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") rw = _stamped_store(tmp_path / "rw.sqlite", "confs:x", "confs:y")
composite = CompositeToolSourceStore(members=[("ro", ro), ("rw", rw)], default="rw") composite = CompositeToolSourceStore(members=[("ro", ro), ("rw", rw)], default="rw")
assert composite.index_is_fresh() is False 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): 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 # Stamped token and probe value disagree, but a read-only store is
# trusted whenever its index loads — the probe only feeds the watcher. # 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 assert ro.index_is_fresh() is True
def test_read_only_store_without_index_is_not_fresh(tmp_path): def test_read_only_store_without_index_is_not_fresh(tmp_path):
SqlAlchemyToolSourceStore(url=f"sqlite:///{tmp_path}/ro.sqlite") SqlAlchemyToolSourceStore(url=f"sqlite:///{tmp_path}/ro.sqlite")
ro = SqlAlchemyToolSourceStore( 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 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): 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") rw = SqlAlchemyToolSourceStore(url=f"sqlite:///{tmp_path}/rw.sqlite")
composite = CompositeToolSourceStore(members=[("ro", ro), ("rw", rw)], default="rw") composite = CompositeToolSourceStore(members=[("ro", ro), ("rw", rw)], default="rw")
assert composite.index_is_fresh() is None assert composite.index_is_fresh() is None