Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added ability to start a job immediately #73

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bugs181
Copy link

@bugs181 bugs181 commented May 31, 2020

Closes #71

Example:

q.pushImmediate(job)
Full code example to show how this would work.
const queue = require('queue')

const q = queue({
  autostart: true,
  timeout: 60 * 1000,
  concurrency: 2,
})

q.on('end', err => {
  console.log('All jobs have finished.')
})

q.on('success', function (result, job) {
  console.log('Job', job.id, 'Start immediately:', job.isImmediate)
})

q.on('timeout', (next, job) => {
  console.log('Job has timed out', job.id)
  next()
})

q.on('error', () => {
  console.log('A job errored out.. but will it continue?')
})

function worker (wait) {
  return function (cb) {
    setTimeout(() => cb(), wait * 1000)
  }
}

// Add 10 jobs that respect concurrency limits
// Jobs finish after 5 seconds
for (var i = 0; i < 10; i++) {
  var w = worker(5)
  w.id = i
  w.isImmediate = false
  q.push(w)
}

// Add two jobs that start immediately.
// Jobs finish immediately
for (var i = 10; i < 12; i++) {
  var w = worker(0)
  w.id = i
  w.isImmediate = true
  q.pushImmediate(w)
}

// Keep adding jobs every 2 seconds
// Jobs finish immediately
let wlen = 12
setInterval(() => {
  var w = worker(0)
  w.id = wlen
  w.isImmediate = true
  q.pushImmediate(w)
  wlen++
}, 2 * 1000)

Output:

Job 10 Start immediately: true
Job 11 Start immediately: true
Job 12 Start immediately: true
Job 13 Start immediately: true
Job 0 Start immediately: false
Job 1 Start immediately: false
Job 14 Start immediately: true
Job 15 Start immediately: true
Job 2 Start immediately: false
Job 3 Start immediately: false
Job 16 Start immediately: true
Job 17 Start immediately: true
Job 18 Start immediately: true
Job 4 Start immediately: false
Job 5 Start immediately: false
Job 19 Start immediately: true
Job 20 Start immediately: true

Github ignored a change in the file, that's a first...
@coveralls
Copy link

Coverage Status

Coverage decreased (-2.4%) to 97.561% when pulling 1e9a5c2 on bugs181:pushImmediate into 9ca37f0 on jessetane:master.

@jessetane
Copy link
Owner

Thanks for the detailed explanation in the issue. How do you feel about skipping the new pushImmediate method and doing just the new start param? I find the name misleading since it unshifts rather than pushes the queue. Also is it possible to get test coverage back to 100%?

@bugs181
Copy link
Author

bugs181 commented Jun 12, 2020

How do you feel about skipping the new pushImmediate method and doing just the new start param?

Good catch, I was assuming the job needed to be added to the top of the queue but after inspecting more of the code, I don't see the necessity in the short term. However, I'd prefer to keep both features in just because it could be easier from an API standpoint to use. In addition it would also future proof the library in case more code is ever added with additional checks. (Job priority features come to mind here.) Without these two features, a job with an immediate priority and start the queue might look something like this:

queue.unshift(job)
queue.start(null, true)

To an end user, that looks mighty confusing. A much prettier alternative (pending the naming convention) is:

queue.addImmediateJobAndStart(job) // Temporarily named `pushImmediate`

I find the name misleading since it unshifts rather than pushes the queue.

I agree. I'm open to change the name. I couldn't think of anything else off the top of my head. What are your thoughts on startImmediate, although that could be confused with start? Another option is addImmediate. I'd really like the name to imply that this job takes priority and will be shoved into the top of the queue and started immediately.


Also is it possible to get test coverage back to 100%?

I haven't had the opportunity to play with tape yet. Personally, I find Tape overly complicated. I'm used to testing with Wallaby and Mocha. What are your thoughts of a PR on moving over to Mocha?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: Immediately start a job
3 participants