mirror of
https://github.com/Crosstalk-Solutions/project-nomad.git
synced 2026-10-17 05:14:19 +02:00
fix(downloads): report a retrying model download as queued, not failed
Model downloads were the only type whose status ignored the BullMQ state fetchJobsWithStates() already tags each job with. Status was read off `failedReason` instead, and BullMQ keeps `failedReason` on a job it has moved to `delayed` for another attempt, so a download retrying while Ollama starts up (Easy Setup) showed as a permanent failure with a Retry button. It now reports the tagged state, which the downloads list already renders as "queued". DownloadModelJob's readiness guard also could never fire: getModels() throws before returning while Ollama isn't up, and never returns null. The throw is now caught, so the retry carries the intended "Ollama service not ready yet" instead of the raw init error. Closes #1311
This commit is contained in:
parent
54ee30a449
commit
24348a9938
|
|
@ -47,11 +47,14 @@ export class DownloadModelJob {
|
|||
|
||||
const ollamaService = new OllamaService()
|
||||
|
||||
// Even if no models are installed, this should return an empty array if ready
|
||||
const existingModels = await ollamaService.getModels()
|
||||
if (!existingModels) {
|
||||
// getModels() throws while Ollama is still starting — Easy Setup dispatches model
|
||||
// downloads without waiting for the install — so treat that as "not ready" and let
|
||||
// the retry backoff wait it out, rather than surfacing the raw init error (#1311)
|
||||
try {
|
||||
await ollamaService.getModels()
|
||||
} catch (error) {
|
||||
logger.warn(
|
||||
`[DownloadModelJob] Ollama service not ready yet for model ${modelName}. Will retry...`
|
||||
`[DownloadModelJob] Ollama service not ready yet for model ${modelName} (${error instanceof Error ? error.message : error}). Will retry...`
|
||||
)
|
||||
throw new Error('Ollama service not ready yet')
|
||||
}
|
||||
|
|
|
|||
|
|
@ -97,13 +97,16 @@ export class DownloadService {
|
|||
}
|
||||
})
|
||||
|
||||
const modelDownloads = modelTagged.map(({ job }) => ({
|
||||
// Status comes from the tagged BullMQ state like every other type. BullMQ keeps
|
||||
// `failedReason` on a job it has moved to `delayed` for another attempt, so
|
||||
// reading status off `failedReason` showed a retrying download as failed (#1311).
|
||||
const modelDownloads = modelTagged.map(({ job, state }) => ({
|
||||
jobId: job.id!.toString(),
|
||||
url: job.data.modelName || 'Unknown Model',
|
||||
progress: parseInt(job.progress.toString(), 10),
|
||||
filepath: job.data.modelName || 'Unknown Model',
|
||||
filetype: 'model',
|
||||
status: (job.failedReason ? 'failed' : 'active') as 'active' | 'failed',
|
||||
status: state,
|
||||
failedReason: job.failedReason || undefined,
|
||||
}))
|
||||
|
||||
|
|
|
|||
61
admin/tests/unit/download_model_status.spec.ts
Normal file
61
admin/tests/unit/download_model_status.spec.ts
Normal file
|
|
@ -0,0 +1,61 @@
|
|||
/**
|
||||
* Tests for how model downloads report their status in the downloads list.
|
||||
*
|
||||
* BullMQ keeps `failedReason` on a job it has moved to `delayed` for another
|
||||
* attempt, so a model download retrying while Ollama starts (Easy Setup) used
|
||||
* to render as a permanent failure (#1311). Status must come from the queue
|
||||
* state the job was fetched from, as it already does for file downloads.
|
||||
*
|
||||
* A Japa spec: DownloadService imports transmit and the Adonis logger at module
|
||||
* scope. Driven through a fake queue, so no Redis is needed. Run with
|
||||
* `node ace test unit`.
|
||||
*/
|
||||
import assert from 'node:assert/strict'
|
||||
import { test } from '@japa/runner'
|
||||
|
||||
import { DownloadService } from '../../app/services/download_service.js'
|
||||
import { DownloadModelJob } from '../../app/jobs/download_model_job.js'
|
||||
|
||||
type FakeJob = { id: string; data: Record<string, unknown>; progress: number; failedReason?: string }
|
||||
|
||||
/** A QueueService whose model queue holds the given jobs by state; every other queue is empty. */
|
||||
function fakeQueueService(modelJobs: Partial<Record<'waiting' | 'active' | 'delayed' | 'failed', FakeJob[]>>) {
|
||||
return {
|
||||
getQueue: (name: string) => ({
|
||||
getJobs: async ([state]: string[]) =>
|
||||
name === DownloadModelJob.queue ? (modelJobs as Record<string, FakeJob[]>)[state] ?? [] : [],
|
||||
}),
|
||||
} as any
|
||||
}
|
||||
|
||||
const job = (id: string, failedReason?: string): FakeJob => ({
|
||||
id,
|
||||
data: { modelName: `model-${id}` },
|
||||
progress: 0,
|
||||
failedReason,
|
||||
})
|
||||
|
||||
test('model download: a retrying job with a failedReason reports delayed, not failed', async () => {
|
||||
const service = new DownloadService(
|
||||
fakeQueueService({ delayed: [job('1', 'Ollama service not ready yet')] })
|
||||
)
|
||||
const [download] = await service.listDownloadJobs('model')
|
||||
assert.equal(download.status, 'delayed')
|
||||
// The reason is still passed through for display
|
||||
assert.equal(download.failedReason, 'Ollama service not ready yet')
|
||||
})
|
||||
|
||||
test('model download: each queue state is reported as itself', async () => {
|
||||
const service = new DownloadService(
|
||||
fakeQueueService({
|
||||
waiting: [job('w')],
|
||||
active: [job('a')],
|
||||
delayed: [job('d', 'retrying')],
|
||||
failed: [job('f', 'gave up')],
|
||||
})
|
||||
)
|
||||
const byId = Object.fromEntries(
|
||||
(await service.listDownloadJobs('model')).map((d) => [d.jobId, d.status])
|
||||
)
|
||||
assert.deepEqual(byId, { w: 'waiting', a: 'active', d: 'delayed', f: 'failed' })
|
||||
})
|
||||
Loading…
Reference in New Issue
Block a user