queue_classic
queue_classic copied to clipboard
QueueClassic
add `Worker#work_off` and `rake qc:work_off`.
This provides a way to work off all the jobs in the queue and exit afterwards.
Closes #191.
:+1:
work_off seems a bit obtuse. If I could go back in history I'd propose qc:start for the daemon task and qc:work for this work-and-exit task. However, assuming we don't want to break the existing task name, I don't have a better suggestion besides something like complete, finish or clear. Actually, I think I like qc:clear best?
I would expect a task named qc:clear to delete of the tasks from the queue, not process them. I'd agree that qc:start and qc:work would be better, but unfortunately, we can't change that. I think work_off is fine for now.
The code is fine, but I worry about how this project is evolving. QC was originally designed (as far as I understand) to have a simple and limited interface for workers. This would allow users of QC to create their own subclass of the worker class and override some method.
It seems that over the year, the size of the worker interface has kept growing, making it less approachable to override.
For instance, if someone overrode the Worker#work method, he now needs to know that he should all override the Worker#work_off method. If he doesn't and he doesn't use the rake task, then it doesn't matter. But the fact of the matter is that you now have a rake task in your application that doesn't work or has unknown behaviour.
Something to think about I guess.
@smathieu I agree with everything you say. I also do think that this functionality should be part of the limited interface we provide.
The PR was created to be fully backwards compatible but as you pointed out, this is a tradeoff in other areas. Depending on wether this lands in a major or minor we might have more flexibility to rework the internals. I'd like to see a more approachable interface as well.
@jipiboily any thoughts on a roadmap?
I'm on the fence on that one. I don't have a strong opinion, but it doesn't feel like something that most people will use.
The more I think about qc, the more I think it should keep a tight scope, stay easily approachable and modifiable. To achieve this, I think we should mostly add stuff that will be used by the majority of the people using qc.
Are there any other use cases you can think of? The original one (using this on Heroku to avoid paying for a full dyno) seems like something very few people will do, and it's easy to work around it.
And by the way, my life changed a bit in the last couple months, you might have seen I don't have much time to work on qc or any open source stuff, and I don't think this will change anytime soon. Basically, don't wait for me if you want to be able to move at a decent pace :)
I still feel we should put this in. Some use-cases I've came across where this would be very helpful.
- For very low-volume projects it might be easier to configure a cron-job using
#work_ofthan to manage background processes. This would be a very easy way to get started with background jobs. - In integration tests when you want to run all queued_jobs before continuing without having to manage a worker process.
This makes sense and has precedent: delayed_job has a jobs:workoff task: https://github.com/collectiveidea/delayed_job#running-jobs
Agreed, a cron job is a good example where this feature would be needed. Another would be spinning up an expensive host twice a day to drain the queue. I'd like to see this feature.
That said, I agree with @smathieu... It's worth worrying if QC's internals are getting weirder. For example, in this PR, lock_job and lock_job_no_wait seem inside-out, and work and work_off seem like copy pasta.
Is it OK to make minor changes to lock_job and work's method signatures to clean this up? I don't imagine anyone would be overriding them.
But maybe it's not OK... Since the docs recommend overriding Worker to manage connections (!), and nobody put any private statements anywhere (!!), this project might have guaranteed that the Worker class will never change... If that's the case, I suppose this PR is the best that can be done and could be merged as-is.
@senny has convinced me that this is valuable, but I'm still not convinced this interface is the right one. Maybe we can limit the scope of change by making the lock_job method take an optional options have. e.g. lock_job(wait: false)
@bronson @senny not sure if y'all use this any more - if you do and you wanna work on this lmk, if not I might
In integration tests when you want to run all queued_jobs before continuing without having to manage a worker process.
I'm not sure if queue_classic already addressed this in the past few years, but que has Que.run_synchronously = true which IMO is a nice solution to this.
Our Que specs use this helper:
def run_que_synchronously
Que.run_synchronously = true
yield
ensure
Que.run_synchronously = false
end
it "runs jobs inline without needing a worker" do
run_que_synchronously do
Job.enqueue()
OtherJob.enqueue()
end
end
