mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-09-21 13:50:20 +08:00
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:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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 —
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user