fix(qmd): make removal wait for the daemon and survive partial failures
Removal reads the daemon PID before --stop and waits up to 10 s for it to exit. A model file that cannot be deleted stays recorded in the state for a retry instead of aborting removal, and a failing models block restore only warns. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
0c203a3905
commit
7ebd61f35b
2 files changed
+111
-7
No files matched your search
+50
-7
@@ -9,6 +9,7 @@ import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
import time
|
||||
|
||||
PACKAGE = '@tobilu/qmd'
|
||||
VERSION = '2.8.3'
|
||||
@@ -25,6 +26,27 @@ def run(arguments, env=None, timeout=1800, check=True):
|
||||
return result.stdout
|
||||
|
||||
|
||||
def process_alive(pid):
|
||||
if WINDOWS: # os.kill(pid, 0) would send CTRL_C_EVENT on Windows
|
||||
import ctypes
|
||||
kernel32 = ctypes.windll.kernel32
|
||||
handle = kernel32.OpenProcess(0x1000, False, pid) # PROCESS_QUERY_LIMITED_INFORMATION
|
||||
if not handle:
|
||||
return False
|
||||
code = ctypes.c_ulong()
|
||||
try:
|
||||
return bool(kernel32.GetExitCodeProcess(handle, ctypes.byref(code))) and code.value == 259 # STILL_ACTIVE
|
||||
finally:
|
||||
kernel32.CloseHandle(handle)
|
||||
try:
|
||||
os.kill(pid, 0)
|
||||
except ProcessLookupError:
|
||||
return False
|
||||
except PermissionError:
|
||||
return True
|
||||
return True
|
||||
|
||||
|
||||
def decide(results):
|
||||
for device in ('gpu', 'cpu'):
|
||||
if any(r.get('device') == device and r.get('passed') is True for r in results):
|
||||
@@ -182,11 +204,20 @@ class Setup:
|
||||
self.say(f'QMD local search enabled on {state["device"].upper()}.')
|
||||
|
||||
def remove_all(self, state):
|
||||
daemon = self.scripts / 'daemon.json'
|
||||
try:
|
||||
pid = json.loads(daemon.read_text(encoding='utf-8')).get('pid') if daemon.is_file() else None
|
||||
except (OSError, ValueError, AttributeError):
|
||||
pid = None
|
||||
try:
|
||||
if self.scripts.joinpath('qmd-search.mjs').exists():
|
||||
self.node('qmd-search.mjs', '--stop', timeout=60)
|
||||
except (ValueError, subprocess.SubprocessError):
|
||||
except (ValueError, OSError, subprocess.SubprocessError):
|
||||
pass
|
||||
if isinstance(pid, int) and pid > 0:
|
||||
deadline = time.monotonic() + 10 # the daemon must release the model files and the package
|
||||
while process_alive(pid) and time.monotonic() < deadline:
|
||||
time.sleep(0.2)
|
||||
collections = self.scripts / 'collections.json'
|
||||
names = json.loads(collections.read_text(encoding='utf-8')) if collections.exists() else []
|
||||
for name in names:
|
||||
@@ -197,19 +228,31 @@ class Setup:
|
||||
pass
|
||||
if state['models_block'] and self.scripts.joinpath('qmd-config.mjs').exists():
|
||||
previous = state.get('previous_models')
|
||||
if previous is not None:
|
||||
self.node('qmd-config.mjs', 'restore-models', json.dumps(previous))
|
||||
else:
|
||||
self.node('qmd-config.mjs', 'unset-models')
|
||||
try:
|
||||
if previous is not None:
|
||||
self.node('qmd-config.mjs', 'restore-models', json.dumps(previous))
|
||||
else:
|
||||
self.node('qmd-config.mjs', 'unset-models')
|
||||
except (ValueError, OSError, subprocess.SubprocessError) as error:
|
||||
print(f'QMD local search: could not restore the models block in index.yml: {error}', file=sys.stderr)
|
||||
kept = []
|
||||
for name in state['owned_models']:
|
||||
path = self.models / name
|
||||
if name == Path(name).name and path.is_file() and not path.is_symlink():
|
||||
path.unlink()
|
||||
try:
|
||||
path.unlink()
|
||||
except OSError as error:
|
||||
print(f'QMD local search: could not delete model {name}; the next removal retries it: {error}', file=sys.stderr)
|
||||
kept.append(name)
|
||||
if state['owned_package']:
|
||||
self.run(['npm', 'uninstall', '--global', PACKAGE])
|
||||
if self.scripts.exists() and not self.scripts.is_symlink():
|
||||
shutil.rmtree(self.scripts)
|
||||
self.state_path.unlink(missing_ok=True)
|
||||
if kept:
|
||||
state.update(clients=[], owned_models=kept, owned_package=False, models_block=False, previous_models=None, files={})
|
||||
atomic_json(self.state_path, state)
|
||||
else:
|
||||
self.state_path.unlink(missing_ok=True)
|
||||
|
||||
def disable(self, clients):
|
||||
state = self.load()
|
||||
|
||||
@@ -290,6 +290,67 @@ class SetupTest(unittest.TestCase):
|
||||
self.setup(FakeRunner(installed=True, fail=('server.py',))).enable(['claude'])
|
||||
self.assertFalse(legacy.exists())
|
||||
|
||||
def test_removal_waits_for_the_daemon_to_exit_before_deleting_models(self):
|
||||
runner = FakeRunner()
|
||||
setup = self.setup(runner)
|
||||
setup.enable(['claude'])
|
||||
(self.scripts_dir() / 'daemon.json').write_text(json.dumps({'port': 1, 'token': 't', 'pid': 4242}))
|
||||
checks = []
|
||||
|
||||
def alive(pid):
|
||||
checks.append((pid, any('--stop' in c for c in runner.calls), any('npm uninstall' in ' '.join(c) for c in runner.calls)))
|
||||
return len(checks) < 3
|
||||
with mock.patch.object(qmd, 'process_alive', alive):
|
||||
setup.disable(['claude'])
|
||||
self.assertEqual(checks, [(4242, True, False)] * 3, 'the PID is polled after --stop and before the uninstall')
|
||||
self.assertIn(['npm', 'uninstall', '--global', '@tobilu/qmd'], runner.calls)
|
||||
|
||||
def test_process_alive(self):
|
||||
self.assertTrue(qmd.process_alive(os.getpid()))
|
||||
child = subprocess.Popen([sys.executable, '-c', 'pass'])
|
||||
child.wait()
|
||||
self.assertFalse(qmd.process_alive(child.pid))
|
||||
|
||||
def test_undeletable_model_keeps_state_for_a_retry(self):
|
||||
runner = FakeRunner()
|
||||
setup = self.setup(runner)
|
||||
setup.enable(['claude'])
|
||||
state = self.state()
|
||||
models = self.home / '.cache/qmd/models'
|
||||
(models / 'locked.gguf').write_text('x')
|
||||
(models / 'free.gguf').write_text('x')
|
||||
state['owned_models'] = ['free.gguf', 'locked.gguf']
|
||||
(self.home / '.my-ai-configuration/qmd.json').write_text(json.dumps(state))
|
||||
real = Path.unlink
|
||||
|
||||
def unlink(path, missing_ok=False):
|
||||
if path.name == 'locked.gguf':
|
||||
raise PermissionError('in use')
|
||||
real(path, missing_ok=missing_ok)
|
||||
with mock.patch.object(qmd.Path, 'unlink', unlink):
|
||||
setup.disable(['claude'])
|
||||
state = self.state()
|
||||
self.assertEqual(state['owned_models'], ['locked.gguf'])
|
||||
self.assertFalse(state['owned_package'])
|
||||
self.assertFalse(state['models_block'])
|
||||
self.assertFalse((models / 'free.gguf').exists())
|
||||
self.assertIn(['npm', 'uninstall', '--global', '@tobilu/qmd'], runner.calls)
|
||||
self.assertFalse(self.scripts_dir().exists())
|
||||
runner.calls.clear()
|
||||
setup.disable(['claude'])
|
||||
self.assertFalse((models / 'locked.gguf').exists())
|
||||
self.assertFalse((self.home / '.my-ai-configuration/qmd.json').exists())
|
||||
self.assertFalse(any('npm uninstall' in ' '.join(c) for c in runner.calls), 'a retry never uninstalls a later foreign package')
|
||||
|
||||
def test_failing_models_block_restore_does_not_stop_removal(self):
|
||||
runner = FakeRunner(fail=('unset-models',))
|
||||
setup = self.setup(runner)
|
||||
setup.enable(['claude'])
|
||||
setup.disable(['claude'])
|
||||
self.assertIn(['npm', 'uninstall', '--global', '@tobilu/qmd'], runner.calls)
|
||||
self.assertFalse(self.scripts_dir().exists())
|
||||
self.assertFalse((self.home / '.my-ai-configuration/qmd.json').exists())
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
Reference in new issue
Block a user