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

Allow Active Record collection tasks to specify a batch size. #1037

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

adrianna-chang-shopify
Copy link
Contributor

Closes: #890

This PR adds a task DSL for specifying the batch size on an Active Record Relation collection. Currently, job-iteration will default to fetching 100 records with active_record_on_records, which is rather small for a table with many thousands of rows. Task authors can now customize the batch size per task with a collection_batch_size method.

Copy link
Contributor

@nvasilevski nvasilevski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

README.md Outdated
@@ -156,6 +156,30 @@ module Maintenance
end
```

#### Customizing the Batch Size

The batch size can be customized for tasks that process Active Record Relations.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the readme, we mention in the batch collection section that regular active record relations are also batched. But I think we should make it clear here since it's at the top of the readme just below the basics. Also there's some potential confusion with the verb here ("that process Active Record Relations"), since the method is called process and we process records when the collection is a relation.

Maybe something along the lines of:

When processing records from an Active Record Relation, records are fetched in batches internally, before being passed each to the process method.

And then the rest about configuring the batch size.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great call out, wording tweaked!

@adrianna-chang-shopify adrianna-chang-shopify merged commit c3893d3 into main Jun 19, 2024
41 checks passed
@adrianna-chang-shopify adrianna-chang-shopify deleted the custom-relation-limit-size branch June 19, 2024 18:57
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.

How do we set limit?
3 participants