chaps-io / gush

Fast and distributed workflow runner using ActiveJob and Redis
MIT License
1.03k stars 103 forks source link

Assign unique names for jobs with the same class #22

Closed ferusinfo closed 8 years ago

ferusinfo commented 8 years ago

Hey, like we've discussed in https://github.com/chaps-io/gush/issues/21, I need to have unique names for jobs that run the same class with different params.

Here is my solution to the problem - I've also updated rspec tests for it (all green now), as well as added mine that is using the new nameize_payloads! method. What it does is changing the way payloads are delivered - they can be either delivered as an array of strings or hashes, depending on how detailed your payloads needs to be:

#normal approach
payloads['DummyJob'] => ['one','two','three']

#hashes approach
payloads['DummyJob'] => [{:id => 'DummyJob-1',:payload => 'one'}]

Hope that you can merge this, so everyone can use it.

pokonski commented 8 years ago

Awesome :) I'm glad you took a shot at this, overall looks great, since payloads are a bit tricky, but I have some suggestions left in the commits.

ferusinfo commented 8 years ago

I've added a new commit with changes that you've proposed - can you take a look at it?

pokonski commented 8 years ago

Great! I'll test it on my projects on the weekend and merge then :)

ferusinfo commented 8 years ago

:+1: I will use my branch in my project production env for the time being ;)

ferusinfo commented 8 years ago

@pokonski just FYI - I am now running this on my production servers for almost 2 days (multiple same class named jobs with different parameters + summary job with Slack hook) - no problems atm.

The only thing that I will need to add myself is better failure handling - the inability to requeue the job in sidekiq is painful (especially that I rely on multiple http requests that might fail at some point).

pokonski commented 8 years ago

Awesome, had a look at it and looks great :+1: Much appreciated! Tiny code style details, but I will change them after merging :)