refactor(registry): make saves last-write-wins, ignore expected_revision
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -25,7 +25,6 @@ from .errors import (
|
||||
CREDENTIAL_NOT_CONFIGURED,
|
||||
MODEL_CONFIGURATION_CHANGED,
|
||||
MODEL_DISABLED,
|
||||
REGISTRY_REVISION_CONFLICT,
|
||||
RUN_CREDENTIAL_REVISION_UNAVAILABLE,
|
||||
ModelRegistryError,
|
||||
)
|
||||
@@ -250,16 +249,17 @@ class ModelRuntimeStore:
|
||||
credential_writes: list[CredentialWrite] | None = None,
|
||||
validate: SaveValidator | None = None,
|
||||
) -> RegistryV4:
|
||||
"""Compare-and-swap the registry inside one ``BEGIN IMMEDIATE`` transaction.
|
||||
"""Save the registry inside one ``BEGIN IMMEDIATE`` transaction.
|
||||
|
||||
Credential versions are written first (immutable, with incrementing
|
||||
revisions), then the optional ``validate`` hook runs the section 9.2
|
||||
save-time checks against the post-write state (so the verification
|
||||
five-tuple sees the new credential revisions), then
|
||||
the registry row is replaced with ``revision + 1``. A bootstrap
|
||||
document atomically turns ``active`` the first time it carries an
|
||||
enabled model, a valid primary, and a satisfied auth reference (4.3).
|
||||
Any failure rolls the whole transaction back.
|
||||
``expected_revision`` is accepted for API compatibility but ignored:
|
||||
saves are last-write-wins and the stored revision is always
|
||||
``current + 1``. Credential versions are written first (immutable,
|
||||
with incrementing revisions), then the optional ``validate`` hook
|
||||
runs the section 9.2 save-time checks against the post-write state,
|
||||
then the registry row is replaced. A bootstrap document atomically
|
||||
turns ``active`` the first time it carries an enabled model, a valid
|
||||
primary, and a satisfied auth reference (4.3). Any failure rolls the
|
||||
whole transaction back.
|
||||
"""
|
||||
now = int(time.time())
|
||||
with self._lock:
|
||||
@@ -271,17 +271,6 @@ class ModelRuntimeStore:
|
||||
).fetchone()
|
||||
current_revision = int(row[0]) if row is not None else 1
|
||||
current_state = str(row[1]) if row is not None else "bootstrap"
|
||||
if current_revision != expected_revision:
|
||||
raise ModelRegistryError(
|
||||
REGISTRY_REVISION_CONFLICT,
|
||||
"registry revision conflict; reload and retry.",
|
||||
details=[
|
||||
{
|
||||
"path": "expected_revision",
|
||||
"code": REGISTRY_REVISION_CONFLICT,
|
||||
}
|
||||
],
|
||||
)
|
||||
for write in credential_writes or []:
|
||||
self._write_credential_version(
|
||||
connection, write.credential_id, write.secret_value, now=now
|
||||
|
||||
@@ -11,7 +11,6 @@ import pytest
|
||||
from EvoScientist.model_registry.errors import (
|
||||
CREDENTIAL_NOT_CONFIGURED,
|
||||
MODEL_DISABLED,
|
||||
REGISTRY_REVISION_CONFLICT,
|
||||
RUN_CREDENTIAL_REVISION_UNAVAILABLE,
|
||||
ModelRegistryError,
|
||||
)
|
||||
@@ -184,15 +183,15 @@ class TestRegistrySave:
|
||||
# The failed save must not leave partial state behind.
|
||||
assert store.load_registry().revision == 1
|
||||
|
||||
def test_stale_expected_revision_conflicts(self, tmp_path):
|
||||
def test_stale_expected_revision_is_ignored(self, tmp_path):
|
||||
store = ModelRuntimeStore(config_dir=tmp_path)
|
||||
store.save_registry(expected_revision=1, registry=_local_registry())
|
||||
with pytest.raises(ModelRegistryError) as excinfo:
|
||||
store.save_registry(expected_revision=1, registry=_local_registry())
|
||||
assert excinfo.value.code == REGISTRY_REVISION_CONFLICT
|
||||
assert store.load_registry().revision == 2
|
||||
# expected_revision is accepted but ignored: last write wins.
|
||||
saved = store.save_registry(expected_revision=1, registry=_local_registry())
|
||||
assert saved.revision == 3
|
||||
assert store.load_registry().revision == 3
|
||||
|
||||
def test_concurrent_saves_single_winner(self, tmp_path):
|
||||
def test_concurrent_saves_last_write_wins(self, tmp_path):
|
||||
first = ModelRuntimeStore(config_dir=tmp_path)
|
||||
first.save_registry(expected_revision=1, registry=_local_registry())
|
||||
|
||||
@@ -214,8 +213,9 @@ class TestRegistrySave:
|
||||
for thread in threads:
|
||||
thread.join(timeout=30)
|
||||
|
||||
assert sorted(outcomes, key=str.lower) == ["ok", REGISTRY_REVISION_CONFLICT]
|
||||
assert first.load_registry().revision == 3
|
||||
assert outcomes == ["ok", "ok"]
|
||||
# Both saves win: 1 (bootstrap) + initial save + two racing saves.
|
||||
assert first.load_registry().revision == 4
|
||||
|
||||
def test_defaults_must_reference_enabled_models(self, tmp_path):
|
||||
store = ModelRuntimeStore(config_dir=tmp_path)
|
||||
|
||||
Reference in New Issue
Block a user