Skip to content

Commit

Permalink
fix(jobs): Fixes creating worker with deleteSuccessfulJobs config set…
Browse files Browse the repository at this point in the history
…ting in JobManager (#11653)

According to the Job documentation, the JobManager's
`deleteSuccessfulJobs ` can be used to decide if one wants successfully
completed jobs from being deleted from the `BackgroundJobs` table.

Keeping job run history around is useful for reporting purposes, such s
hoe many jobs run over time, how many fails, how many successes, etc.

However, the `deleteSuccessfulJobs` was not being correctly passed to
the worker in `createWorker` so the worker always used the default value
-- true -- and always deleted the job run record regardless of
configuration.

This PR fixes this issue by setting the config value when creating the
worker.
  • Loading branch information
dthyresson authored Oct 5, 2024
1 parent cc867c3 commit bef7939
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 0 deletions.
9 changes: 9 additions & 0 deletions .changesets/11653.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
- fix(jobs): Fixes creating worker with deleteSuccessfulJobs config setting in JobManager (#11653) by @dthyresson

According to the Job documentation, the JobManager's `deleteSuccessfulJobs ` can be used to decide if one wants successfully completed jobs from being deleted from the `BackgroundJobs` table.

Keeping job run history around is useful for reporting purposes, such s hoe many jobs run over time, how many fails, how many successes, etc.

However, the `deleteSuccessfulJobs` was not being correctly passed to the worker in `createWorker` so the worker always used the default value -- true -- and always deleted the job run record regardless of configuration.

This PR fixes this issue by setting the config value when creating the worker.
1 change: 1 addition & 0 deletions packages/jobs/src/core/JobManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export class JobManager<
maxRuntime: config.maxRuntime,
sleepDelay: config.sleepDelay,
deleteFailedJobs: config.deleteFailedJobs,
deleteSuccessfulJobs: config.deleteSuccessfulJobs,
processName,
queues: [config.queue].flat(),
workoff,
Expand Down
49 changes: 49 additions & 0 deletions packages/jobs/src/core/__tests__/JobManager.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { describe, expect, vi, it, beforeEach } from 'vitest'

import type { Job, JobDefinition } from '../../types.js'
import type { CreateWorkerArgs } from '../JobManager.js'
import { JobManager } from '../JobManager.js'
import { Scheduler } from '../Scheduler.js'
import { Worker } from '../Worker.js'

import { MockAdapter, mockLogger } from './mocks.js'

Expand Down Expand Up @@ -245,3 +247,50 @@ describe('createJob()', () => {
expect(job).toEqual(jobDefinition)
})
})

describe('createWorker()', () => {
it('creates a worker with a custom configuration', () => {
const mockAdapter = new MockAdapter()
const customLogger = { ...mockLogger, custom: true }
const manager = new JobManager({
adapters: { mock: mockAdapter },
queues: ['default', 'high'] as const,
logger: mockLogger,
workers: [
{
adapter: 'mock' as const,
queue: ['default', 'high'] as const,
count: 1,
logger: customLogger,
maxAttempts: 10,
maxRuntime: 120000,
sleepDelay: 7,
deleteFailedJobs: true,
deleteSuccessfulJobs: false,
},
],
})

const workerArgs: CreateWorkerArgs = {
index: 0,
workoff: true,
clear: true,
processName: 'test-worker-custom-config',
}

const worker = manager.createWorker(workerArgs)

expect(worker).toBeInstanceOf(Worker)
expect(worker).toHaveProperty('adapter', mockAdapter)
expect(worker).toHaveProperty('logger', customLogger)
expect(worker).toHaveProperty('maxAttempts', 10)
expect(worker).toHaveProperty('maxRuntime', 120000)
expect(worker).toHaveProperty('sleepDelay', 7000)
expect(worker).toHaveProperty('deleteFailedJobs', true)
expect(worker).toHaveProperty('deleteSuccessfulJobs', false)
expect(worker).toHaveProperty('processName', 'test-worker-custom-config')
expect(worker).toHaveProperty('queues', ['default', 'high'])
expect(worker).toHaveProperty('workoff', true)
expect(worker).toHaveProperty('clear', true)
})
})

0 comments on commit bef7939

Please sign in to comment.