fix: serialize deployment revision writes
This commit is contained in:
+20
-13
@@ -3,6 +3,7 @@ from __future__ import annotations
|
|||||||
import json
|
import json
|
||||||
import re
|
import re
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
from threading import RLock
|
||||||
|
|
||||||
from .models import WorkflowArtifact, WorkflowDeployment
|
from .models import WorkflowArtifact, WorkflowDeployment
|
||||||
|
|
||||||
@@ -65,6 +66,7 @@ class FileWorkflowArtifactStore(WorkflowArtifactStore):
|
|||||||
|
|
||||||
def __init__(self, root: Path) -> None:
|
def __init__(self, root: Path) -> None:
|
||||||
self.root = root
|
self.root = root
|
||||||
|
self._lock = RLock()
|
||||||
self.artifacts_dir.mkdir(parents=True, exist_ok=True)
|
self.artifacts_dir.mkdir(parents=True, exist_ok=True)
|
||||||
self.deployments_dir.mkdir(parents=True, exist_ok=True)
|
self.deployments_dir.mkdir(parents=True, exist_ok=True)
|
||||||
|
|
||||||
@@ -111,20 +113,25 @@ class FileWorkflowArtifactStore(WorkflowArtifactStore):
|
|||||||
return self.get_artifact(artifact_id, max(versions))
|
return self.get_artifact(artifact_id, max(versions))
|
||||||
|
|
||||||
def save_deployment(self, deployment: WorkflowDeployment) -> None:
|
def save_deployment(self, deployment: WorkflowDeployment) -> None:
|
||||||
path = self._deployment_path(deployment.id)
|
# Revision advancement is store-owned and must cover the read,
|
||||||
if path.exists():
|
# increment, and write as one process-local transition. Callers
|
||||||
existing = WorkflowDeployment.model_validate_json(
|
# intentionally continue sending the existing deployment payload
|
||||||
path.read_text(encoding="utf-8")
|
# without a mandatory expected-revision field.
|
||||||
|
with self._lock:
|
||||||
|
path = self._deployment_path(deployment.id)
|
||||||
|
if path.exists():
|
||||||
|
existing = WorkflowDeployment.model_validate_json(
|
||||||
|
path.read_text(encoding="utf-8")
|
||||||
|
)
|
||||||
|
deployment = deployment.model_copy(
|
||||||
|
update={"revision": existing.revision + 1}
|
||||||
|
)
|
||||||
|
elif deployment.revision != 1:
|
||||||
|
deployment = deployment.model_copy(update={"revision": 1})
|
||||||
|
path.write_text(
|
||||||
|
json.dumps(deployment.model_dump(mode="json"), indent=2),
|
||||||
|
encoding="utf-8",
|
||||||
)
|
)
|
||||||
deployment = deployment.model_copy(
|
|
||||||
update={"revision": existing.revision + 1}
|
|
||||||
)
|
|
||||||
elif deployment.revision != 1:
|
|
||||||
deployment = deployment.model_copy(update={"revision": 1})
|
|
||||||
path.write_text(
|
|
||||||
json.dumps(deployment.model_dump(mode="json"), indent=2),
|
|
||||||
encoding="utf-8",
|
|
||||||
)
|
|
||||||
|
|
||||||
def get_deployment(self, deployment_id: str) -> WorkflowDeployment:
|
def get_deployment(self, deployment_id: str) -> WorkflowDeployment:
|
||||||
path = self._deployment_path(deployment_id)
|
path = self._deployment_path(deployment_id)
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import json
|
import json
|
||||||
|
from concurrent.futures import ThreadPoolExecutor
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
@@ -67,6 +68,32 @@ def test_file_store_round_trips_deployment(tmp_path) -> None:
|
|||||||
assert loaded.binding_map()["context7"] == "context7.personal"
|
assert loaded.binding_map()["context7"] == "context7.personal"
|
||||||
|
|
||||||
|
|
||||||
|
def test_concurrent_deployment_saves_advance_revision_without_lost_updates(
|
||||||
|
tmp_path,
|
||||||
|
) -> None:
|
||||||
|
store = FileWorkflowArtifactStore(tmp_path)
|
||||||
|
store.save_deployment(
|
||||||
|
WorkflowDeployment(
|
||||||
|
id="concurrent.personal",
|
||||||
|
artifact_id="summarize_docs",
|
||||||
|
artifact_version=1,
|
||||||
|
)
|
||||||
|
)
|
||||||
|
updates = [
|
||||||
|
WorkflowDeployment(
|
||||||
|
id="concurrent.personal",
|
||||||
|
artifact_id="summarize_docs",
|
||||||
|
artifact_version=version,
|
||||||
|
)
|
||||||
|
for version in range(2, 10)
|
||||||
|
]
|
||||||
|
|
||||||
|
with ThreadPoolExecutor(max_workers=len(updates)) as executor:
|
||||||
|
list(executor.map(store.save_deployment, updates))
|
||||||
|
|
||||||
|
assert store.get_deployment("concurrent.personal").revision == 9
|
||||||
|
|
||||||
|
|
||||||
def test_file_store_loads_legacy_artifact_and_rewrites_canonical_shape(
|
def test_file_store_loads_legacy_artifact_and_rewrites_canonical_shape(
|
||||||
tmp_path,
|
tmp_path,
|
||||||
) -> None:
|
) -> None:
|
||||||
|
|||||||
Reference in New Issue
Block a user