Gush::Client now delegates the ActiveJob.perform_later call to Gush::Job
#117
Conversation
…ush::Job` This delegation allows for Gush users to easily override the enqueing behavior to their custom job class, e.g. to support additional ActiveJob options or enqueing behaviors (e.g synchronous with `perform_now`).
| @@ -59,6 +59,14 @@ def enqueue! | |||
| @failed_at = nil | |||
| end | |||
|
|
|||
| def enqueue_worker!(options = {}) | |||
| Gush::Worker.set(options).perform_later(workflow_id, name) | |||
There was a problem hiding this comment.
This reads great! Simple and clean abstraction. It feels like no matter what else happens around it this should be the goal for the interface to the Worker class. Kudos!
| Gush::Worker.set(queue: queue).perform_later(*[workflow_id, job.name]) | ||
| end | ||
|
|
||
| options = { queue: configuration.namespace }.merge(job.worker_options) |
There was a problem hiding this comment.
The overriding method approach definitely comes together well. It makes for a concise and flexible interface to the Job class. If I put on my horn-rimmed-dark-tinted critic spectacles (and squint pensively) it does read a little odd to have to reach into the #worker_options method to merge back in to the #enqueue_worker! method on the same worker object.
This is partly semantic, and you could clear it up maybe with a #default_worker_options intermediary, or something similar, but I am not sure it is worth the additional layer without actually trying it.
To get anywhere with another approach would probably have to sacrifice some of the simplicity of this approach, so if the maintainers like it then so do I!
There was a problem hiding this comment.
Thanks for the review and comments!
I originally considered this alternative:
class Gush::Client
def enqueue_job(workflow_id, job)
...
job.enqueue_worker!(queue: configuration.namespace)
end
end
class Gush::Job
def enqueue_worker!(default_options = {})
options = default_options.merge(worker_options)
Gush::Worker.set(options).perform_later(workflow_id, name)
end
endbut I think it's preferable to have Job#enqueue_worker! receive everything it needs without having to do additional work to get options, and allow the extra line of complexity to be in the Client class, since that's not meant to be overridden.
There was a problem hiding this comment.
Looks like you already tried the default semantic and have good reason to avoid it, thanks for the reply!
This delegation allows for Gush users to easily override the enqueing behavior to their custom job class, e.g. to support additional ActiveJob options or enqueing behaviors (e.g synchronous with
perform_now).