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

Support ActiveRecord::Batches::BatchEnumerator as a collection (or don't require a collection at all) #390

Closed
lamp opened this issue Mar 31, 2021 · 2 comments · Fixed by #409

Comments

@lamp
Copy link

lamp commented Mar 31, 2021

Hi, while implementing this job for recent task, I ran into the requirement here that I return a known collection type.

As you can see, to make my job work I had to provide an array with at least one value, it would be nice if I could have done:

module Maintenance  
  class CopyOriginPostalCodeToSampleZipLocationTask < MaintenanceTasks::Task
    def collection
      ReconExtractBatchDetail.in_batches
    end

    def count
      ReconExtractBatchDetail.count
    end

    def process(batch)
        batch.update_all('sample_zip_location = origin_postal_code')
    end
  end
end

It looks like it should "just work" by adding the BatchEnumerator to the allowed list of classes, but I might be missing something, let me know if you have any questions and if needed I'd be happy to work on it with you.

Thanks a lot for the library, it works great other than this small issue.

@adrianna-chang-shopify
Copy link
Contributor

👋 Hi folks! Unfortunately we can't directly use ActiveRecord::Batches::BatchEnumerator because it's not compatible with the way Job Iteration builds Active Record enumerators. That being said, I'm proposing an alternative API that still provides support for batch collections in Tasks, leveraging the mechanism Job Iteration offers (#build_active_record_enumerator_on_batches)

I've opened a more generic batch support issue here: #401. I'm going to close this issue.

@adrianna-chang-shopify
Copy link
Contributor

Reopening, we've discussed the pros of leveraging BatchEnumerator for this feature, and it makes the most sense. We'll look into modifying Job Iteration's API so that we can use in_batches.

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 a pull request may close this issue.

2 participants