lykmapipo / kue-scheduler

A job scheduler utility for kue, backed by redis and built for node.js
246 stars 47 forks source link

fix job:undefined issue #14

Closed risseraka closed 8 years ago

risseraka commented 8 years ago

Hi there!

toJSON is called against the soon-to-be-saved job here, here or here: https://github.com/lykmapipo/kue-scheduler/blob/master/index.js#L558 https://github.com/lykmapipo/kue-scheduler/blob/master/index.js#L683 https://github.com/lykmapipo/kue-scheduler/blob/master/index.js#L779

Here's kue's toJSON for a job: https://github.com/Automattic/kue/blob/master/lib/queue/job.js#L298

Which begets this kind of object:

{
   type: 'job_type',
   data: {
     unique: 'job_type',
     schedule: 'RECCUR'
   },
   priority: 0,
   progress: 0,
   attempts: {
     made: 0,
     remaining: 1,
     max: 1
   },
   reccurInterval: '10 seconds'
 }

By looping against jobDefinition properties: https://github.com/lykmapipo/kue-scheduler/blob/master/index.js#L274-L284 you stumble upon the progress property which is one of a job's method as seen here: https://github.com/Automattic/kue/blob/master/lib/queue/job.js#L386

job.progress() is thus called, which in turn sets progress https://github.com/Automattic/kue/blob/master/lib/queue/job.js#L389 and updated_at https://github.com/Automattic/kue/blob/master/lib/queue/job.js#L395 but ths.id is undefined https://github.com/Automattic/kue/blob/master/lib/queue/job.js#L356 thus creating a new hash at key "q:job:undefined".

A quick fix would be to exclude progress from jobDefinition's forEach as per this pull request.

Let me know what you think.

Cheers!

PS: why is kue only in devDependencies and not plain dependencies? I'm a bit lost.

lykmapipo commented 8 years ago

@risseraka

Thanks.

lykmapipo commented 8 years ago

@risseraka

Am going to add it to dependencies. :100:

risseraka commented 8 years ago

:+1: