Skip to content
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

Add ActiveScheduler.config.check_jobs_descend_from_active_job #22

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AMHOL
Copy link
Contributor

@AMHOL AMHOL commented Apr 1, 2021

When set to false, this will skip the check in ActiveScheduler::ResqueWrapper.wrap which checks whether the job class descends from ActiveJob::Base, this means that we no longer need to call class_name.constantize, which should mean that we can skip loading the whole Rails environment for the resque scheduler rake task and save some memory.

When set to false, this will skip the check in ActiveScheduler::ResqueWrapper.wrap which checks whether the job class descends from ActiveJob::Base, this means that we no longer need to call "class_name.constantize", which should mean that we can skip loading the whole Rails environment for the resque scheduler rake task and save some memory.
@AMHOL
Copy link
Contributor Author

AMHOL commented Apr 1, 2021

From #20

I had also considered adding a configurable guard proc instead of this, to allow for custom matching to wrap jobs rather than it being all-or-nothing, but I couldn't come up with a good name and it got messy when trying to resolve the queue name, which I ended up adding an additional queue_name_resolver config for. I could spend some more time on that approach if you prefer it to this. The config would have ended up being something like:

ActiveScheduler.configure do |config|
  config.guard_with = ->(class_name) { class_name.constantize <= ActiveJob::Base }
  config.queue_name_resolver = ->(class_name) { class_name.constantize.queue_name }
end

With this approach, we would lose the ability to raise an ActiveScheduler::QueueConfigMissingError exception when the queue name is not configured and we don't check the job class descends from ActiveJob::Base

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 0db79fc on AMHOL:feature/check_jobs_descend_from_active_job-config into ee3a38c on JustinAiken:master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants