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

Batch docs in README suggest Batch#add is a class method, but it's not #912

Closed
dmolesUC opened this issue Apr 3, 2023 · 3 comments
Closed

Comments

@dmolesUC
Copy link

dmolesUC commented Apr 3, 2023

Based on this example from the README, I've written the following:

def schedule_jobs(task)
  batch = GoodJob::Batch.add do
    Holdings::WorldCatJob.perform_later(task) if task.world_cat?
    Holdings::HathiTrustJob.perform_later(task) if task.hathi?
  end
  batch.enqueue(on_finish: Holdings::ResultsJob, task:)
end

However, this fails with NoMethodError: undefined method 'add' for GoodJob::Batch:Class.

Should I instead just write

GoodJob::Batch.enqueue(on_finish: Holdings::ResultsJob, task:) do
  Holdings::WorldCatJob.perform_later(task) if task.world_cat?
  Holdings::HathiTrustJob.perform_later(task) if task.hathi?
end

?

@bensheldon
Copy link
Owner

Whoops! Sorry about that; the readme is incorrect. I'd recommend you structure it this way, using a Batch instance:

def schedule_jobs(task)
  batch = GoodJob::Batch.new(on_finish: Holdings::ResultsJob, task:)
  batch.add do
    Holdings::WorldCatJob.perform_later(task) if task.world_cat?
    Holdings::HathiTrustJob.perform_later(task) if task.hathi?
  end
  batch.enqueue
end

You could also do this:

GoodJob::Batch.enqueue(on_finish: Holdings::ResultsJob, task:) do
  Holdings::WorldCatJob.perform_later(task) if task.world_cat?
  Holdings::HathiTrustJob.perform_later(task) if task.hathi?
end

Or if you wanted to make sure you didn't end up with an empty batch, you could do something like this to move the conditions outside of the batch actions themselves.

batch = GoodJob::Batch.new(on_finish: Holdings::ResultsJob, task:)
batch.add(Holdings::WorldCatJob.new(task)) if task.world_cat?
batch.add(Holdings::HathiTrustJob.new(task)) if task.hathi?
batch.enqueue if batch.active_jobs.present?

@bensheldon
Copy link
Owner

Fixed in f8bd028

@dmolesUC
Copy link
Author

dmolesUC commented Apr 4, 2023

@bensheldon Thanks!

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

No branches or pull requests

2 participants