use enqueue instead of retry_job to reenqueue jobs#32
use enqueue instead of retry_job to reenqueue jobs#32GustavoCaso wants to merge 1 commit intomainfrom
Conversation
GustavoCaso
force-pushed
the
do-not-use-retry-job-to-reenqueue-job
branch
from
ea7c15f to
0b226fb
Compare
GustavoCaso
force-pushed
the
do-not-use-retry-job-to-reenqueue-job
branch
from
0b226fb to
fc5657c
Compare
kirs
left a comment
kirs
left a comment
There was a problem hiding this comment.
I don't have a strong preference, but AFAIK @rafaelfranca recommended we use retry_job for re-enqueueing.
Semantically, I agree that this is not a "retry" but rather a "re-enqueue".
| self.already_in_queue = true if respond_to?(:already_in_queue=) | ||
| retry_job | ||
|
|
||
| enqueue unless @enqueued |
There was a problem hiding this comment.
You no-op enqueueing if @enqueued is set, but what about the rest of side-effects in that method? Why do they need to run if the job was already enqueued?
There was a problem hiding this comment.
Unless I'm missing something you need the defined?(@enqueued) otherwise you will get a Ruby warning.
|
I'm not against this change, if the goal if to make |
3 participants
@kirs I wanted to see if we can remove the call for
retry_joband all the test will pass :)I wanted to remove the use of
retry_jobbecause I plan on overwrite it on core to increase the number of attempts from theRetrymodule, to make sure are not retrying more than they should.