fix(lora): the import review takes a lease beside a restore
The LoRA import review reads the library, so like the maintenance and restore previews it now runs under a request lease: it is refused with 409 busy while a restore runs, and a restore waits for it before it switches the file. Tests pin that a restore is refused while an import runs and that the review, the reviewed import and the admin page's scan are refused while a restore runs.
This commit is contained in:
+4
-1
@@ -3155,7 +3155,10 @@ folders (`TestStaleDownloadSweep`). The legacy `POST /lora/scan` runs the same s
|
||||
event shape; a page that closes its connection stops staging before its next LoRA, and the
|
||||
scan keeps its records hold until staging has stopped, then ends `cancelled` in Activity
|
||||
(`test_a_disconnected_legacy_scan_stops_and_holds_until_it_stopped`) (`test_legacy_scan_events_unchanged_on_success`), holds
|
||||
the registry as "LoRA import (admin page)" and takes no safety backup. `POST /lora/enable`
|
||||
the registry as "LoRA import (admin page)" and takes no safety backup. Both imports are
|
||||
`records` jobs, so a restore (`exclusive`, #249 S8) is refused while one runs and they are
|
||||
refused while a restore runs; the review is a request lease ("LoRA import review"), refused
|
||||
(409 `busy`) during a restore, and a restore waits for it (`LoraImportRestoreTests`). `POST /lora/enable`
|
||||
accepts the optional `expected_revision`. LoraManager detection also searches every
|
||||
`custom_nodes` folder ComfyUI registers through `folder_paths` (#144), main folder first.
|
||||
|
||||
|
||||
@@ -54,6 +54,9 @@ except ImportError: # pragma: no cover - flat test imports
|
||||
from database.pairs import PairDependencyError
|
||||
|
||||
LORA_OPERATION = "lora_import"
|
||||
# The review reads the library: a request lease, so it is refused while a
|
||||
# restore runs and a restore waits for it before switching the file (S8).
|
||||
LORA_PREVIEW_LEASE = "LoRA import review"
|
||||
CATEGORY = lora_import.LORA_CATEGORY
|
||||
SOURCE_CHANGED = (
|
||||
"The LoRA metadata files changed since the review; nothing was changed. "
|
||||
@@ -179,6 +182,13 @@ class LoraImportRoutesMixin:
|
||||
# ── preview ───────────────────────────────────────────────────────
|
||||
|
||||
async def preview_lora_import(self, request):
|
||||
try:
|
||||
async with self._jobs.lease(LORA_PREVIEW_LEASE):
|
||||
return await self._preview_lora_import(request)
|
||||
except JobBusy as busy:
|
||||
return busy_response(busy)
|
||||
|
||||
async def _preview_lora_import(self, request):
|
||||
data, error = await self._read_maintenance_body(request, ())
|
||||
if error is not None:
|
||||
return error
|
||||
|
||||
@@ -43,6 +43,8 @@ from database.category_import import EmptyReplacement
|
||||
from database.maintenance import PlanChanged
|
||||
from database.operations import PairDependencyError, PromptDatabase
|
||||
from py import lora_import, lora_utils
|
||||
from py.api.jobs import RESTORE_JOB_KIND
|
||||
from py.api.lora_import_routes import LORA_PREVIEW_LEASE
|
||||
from py.config import IntegrationConfig, PromptManagerConfig
|
||||
from utils.hashing import generate_prompt_hash
|
||||
|
||||
@@ -810,6 +812,68 @@ def _size(path):
|
||||
return os.path.getsize(path) if os.path.exists(path) else None
|
||||
|
||||
|
||||
class LoraImportRestoreTests(LoraImportAPITestCase):
|
||||
"""The import and a restore (#249 S8) never run beside each other."""
|
||||
|
||||
async def test_a_restore_is_refused_while_the_import_runs(self):
|
||||
self._two_loras()
|
||||
self.gate = threading.Event()
|
||||
_plan, job = await self._apply_reviewed()
|
||||
loop = asyncio.get_running_loop()
|
||||
self.assertTrue(await loop.run_in_executor(None, self.downloading.wait, 10))
|
||||
|
||||
resp = await self.client.request("POST", "/prompt_manager/restore")
|
||||
body = json.loads(await resp.text())
|
||||
self.assertEqual(resp.status, 409)
|
||||
self.assertIn(lora_import.JOB_TITLE, body["error"])
|
||||
# The workspace restore job is refused for the same running import.
|
||||
blocker = self.api._jobs.blocker(RESTORE_JOB_KIND, admission="exclusive")
|
||||
self.assertIs(blocker, job)
|
||||
|
||||
self.gate.set()
|
||||
await job.task
|
||||
self.assertEqual(job.state, "finished")
|
||||
|
||||
async def test_the_import_is_refused_while_a_restore_runs(self):
|
||||
self._two_loras()
|
||||
resp, plan = await self._post(PREVIEW)
|
||||
self.assertEqual(resp.status, 200, plan)
|
||||
before = _snapshot(self.api.db)
|
||||
async with self.api._jobs.hold(
|
||||
RESTORE_JOB_KIND, "Restore library", admission="exclusive"
|
||||
):
|
||||
# A review reads the library: refused, not run beside the switch.
|
||||
resp, body = await self._post(PREVIEW)
|
||||
self.assertEqual(resp.status, 409, body)
|
||||
self.assertEqual(body["code"], "busy")
|
||||
self.assertEqual(body["running"]["title"], "Restore library")
|
||||
resp, body = await self._post(APPLY, {"token": plan["token"]})
|
||||
self.assertEqual(resp.status, 409, body)
|
||||
self.assertEqual(body["code"], "busy")
|
||||
# The admin page's scan says so in its own error event.
|
||||
resp = await self.client.request("POST", SCAN)
|
||||
events = self._sse(await resp.text())
|
||||
self.assertEqual(events[-1]["type"], "error")
|
||||
self.assertIn("Restore library", events[-1]["message"])
|
||||
self.assertEqual(_snapshot(self.api.db), before)
|
||||
self.assertIsNone(self.api._jobs.active("lora_import"))
|
||||
|
||||
async def test_a_running_review_holds_a_lease_a_restore_waits_for(self):
|
||||
self._two_loras()
|
||||
seen = []
|
||||
real_preview = lora_import.preview
|
||||
|
||||
def preview(*args):
|
||||
seen.append(self.api._jobs.leases)
|
||||
return real_preview(*args)
|
||||
|
||||
with patch.object(lora_import, "preview", preview):
|
||||
resp, plan = await self._post(PREVIEW)
|
||||
self.assertEqual(resp.status, 200, plan)
|
||||
self.assertEqual(seen, [{LORA_PREVIEW_LEASE: 1}])
|
||||
self.assertEqual(self.api._jobs.leases, {})
|
||||
|
||||
|
||||
class LoraImportStageTests(unittest.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
|
||||
Reference in New Issue
Block a user