-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tuning retry based on ActiveJob Exceptions implementation #4
base: master
Are you sure you want to change the base?
Conversation
testapp/app/jobs/test_job.rb
Outdated
@@ -1,9 +1,16 @@ | |||
# frozen_string_literal: true | |||
|
|||
class TestJob < ApplicationJob | |||
class CustomError < StandardError; end | |||
|
|||
retry_on(CustomError, wait: 5, attempts: 3, queue: :retry) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what will the redis key look like for the retry
queue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be prefixed with queue:
, queue:retry
in this case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's prefixed here https://github.com/sailor/quiq/blob/master/lib/quiq/scheduler.rb#L47
klass = Object.const_get(payload['job_class']) | ||
args = payload['arguments'] | ||
job = klass.deserialize(payload) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
neat!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: is it a rails/activejob primitive? If so it means that the system won't be usable outside of the rails ecosystem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should use if defined? ActiveJob
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the retry mechanism is a rails/activejob primitive, but the serialize
(which is used in Quiq::Client#push) and deserialize
are probably not.
if defined? ActiveJob
could be done during the server booting, wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes it could probably do the job!
ActiveJob has a built-in retry mechanism.
https://github.com/rails/rails/blob/6-0-stable/activejob/lib/active_job/exceptions.rb#L50-L67
By deserializing the job message properly, we can get the retry working easily with Quiq.