contribsys / faktory_worker_ruby

Faktory worker for Ruby
GNU Lesser General Public License v3.0
214 stars 31 forks source link

Make creating polyglot jobs easier #41

Closed fnordfish closed 4 years ago

fnordfish commented 4 years ago

Allows for easier enqueuing of jobs where the Job processor lives in another system.

Faktory::Job.set(jobtype: 'someFunc', queue: 'some_queue').perform_async("arg1", 2)
Faktory::Job.set(jobtype: 'someFunc', queue: 'some_queue').perform_in(12.minute, "arg1", 2)

MyPolyglotJob = Faktory::Job.set(queue: 'some_q', jobtype: 'someFunc')
MyPolyglotJob.perform_async("arg1", 2)

The enqueuing ruby process does not need to know the Job class. So we can use any jobtype name, as long as the worker process knows how to deal with it. Traditional Job class (mixing in Faktory::Job + SomeJob.perform_async) still enforce their own class name as jobtype.

Also see discussion in #30

mperham commented 4 years ago

Use the set method to specify options so you don’t need the extra Hash argument on the end of perform* and can instead list out the arguments without the wrapping Array.

On Dec 22, 2019, at 05:35, Robert Schulze notifications@github.com wrote:

 This moves the logic of the Faktory::Job mixin perform_* methods onto the Faktory::Job module.

Faktory::Job.perform_async('someFunc', ["arg1"], queue: 'some_queue') Faktory::Job.perform_in(12.minute, 'someFunc', ["arg1"], queue: 'some_queue') The enqueuing ruby process does not need to know the Job class.

without the ruby Job class known to the Also see discussion in #30

You can view, comment on, or merge this pull request online at:

https://github.com/contribsys/faktory_worker_ruby/pull/41

Commit Summary

Move perform_* methods to Job module File Changes

M lib/faktory/job.rb (77) A test/polyglot_test.rb (51) M test/testing_fake_test.rb (33) M test/testing_inline_test.rb (14) Patch Links:

https://github.com/contribsys/faktory_worker_ruby/pull/41.patch https://github.com/contribsys/faktory_worker_ruby/pull/41.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

fnordfish commented 4 years ago

I see where you're going at. But if we'd prefer set, wouldn't it make also sense to not provide perform_async(jobtype, *args) and require Faktory::Job.set(queue: 'some_q', jobtype: 'someFunc').perform_async('some', 'args') instead?

You could even use it as a constant :)

MyPolyglotJob = Faktory::Job.set(queue: 'some_q', jobtype: 'someFunc')

One thing I stumbled upon: To me it looks like the pool is serialized into the job item when given as a faktory_option. Is that on purpose? (https://github.com/contribsys/faktory_worker_ruby/blob/master/lib/faktory/job.rb#L113-L114)

mperham commented 4 years ago

Pool should not be serialized, correct. This PR looks great now. I’m taking the next 10 days off but I will dbl check and merge after the holidays. 😎

fnordfish commented 4 years ago

Sure thing. Enjoy the holidays.

mperham commented 4 years ago

Nicely done and thanks for the well-done tests.