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

[10.x] Allow placing a batch on a chain #48633

Merged
merged 15 commits into from Nov 3, 2023
Merged

Conversation

khepin
Copy link
Contributor

@khepin khepin commented Oct 4, 2023

Context

Where do we use this:

This is a use case we encounter in a couple places at Square where we have a serial process of jobs that need to be handled and one or more of the steps either should be worked on in parallel or are of unknown length when initially triggering the workflow and will create additional jobs, but we need to know when this is finished to ensure we keep the chain going.

Example workflow

An example could be onboarding users from a different, separate system. The steps to do so could be the following:

1. Create user account and base metadata
2. Pull user account additional information / status from remote systems
3. Download e-commerce catalog from service A
    ^ many paginated requests, good use case for Batch.
4. Notify service B or current setup status
5. Precompute indexing data based on data downloaded in step 3
    ^ also a perfect candidate for batching
6. Notify user of account readiness

How it would look like with Laravel today

There is currently not a great way to express something like this in Laravel. The closest we can get to is something like:

// Start with a chain of multiple steps
Bus::chain([
    new AccountInitialize(),
    new GetAccountMetadata(),
    /**
     * This step would make paginated requests to a remote service.
     * Each job would query 1 page and then if given a cursor to a next page,
     * dispatch a job to do the same thing on that next page.
     * 
     * So we're creating a job to start a batch since a batch can dynamically
     * receive new jobs and still track the completion of the entire process. 
     */
    new StartDownloadEcommerceData(), 
])->dispatch();

class StartDownloadEcommerceData extends BaseJob
{
    public function handle()
    {
        Bus::batch([new DownloadEcommerceDataPage(page: 0)]) // if there are more pages, the first job would add another to the batch
            ->then(function () {
                Bus::chain([
                    new NotifyServiceBStatusUpdate(),
                    new StartPrecomputingDownloadedData(),
                ])->dispatch();
            });
    }
}

class StartPrecomputingDownloadedData extends BaseJob
{
    public function handle()
    {
        $b = Bus::batch();
        foreach ($data as $d) {
            $b->add(new PrecomputeData($d));
        }

        $b->then(function () {
            dispatch(new NotifyUserAccountReady());
        });

        $b->dispatch();
    }
}

Proposal

The previous example takes away a lot of clarity from the workflow with different pieces being started / kicked off in multiple places in code. What we would prefer instead is to write something like:

Bus::chain([
    new AccountInitialize(),
    new GetAccountMetadata(),
    Bus::batch([new DownloadEcommerceDataPage(page: 0)]), // if there are more pages, the first job would add another to the batch
    new NotifyServiceBStatusUpdate(),
    Bus::batch([new StartPrecomputingDownloadedData()]), // first job will load the data and add 1 job / item to the batch
    new NotifyUserAccountReady(),
])->dispatch();

This allows us to see in one single place all the actions that will happen as part of the entire workflow. That also makes maintenance much simpler as we don't need to track across many files the downstream dependencies and effects of a given job. Everything is laid out clearly in one central place.

The PR

This PR implements a way to achieve the proposed syntax above. It also allows nesting chains and batches to even deeper levels if needed.

Would love to see if this would make a good addition to Laravel. This is definitely something we'll be using on our side.

@driesvints driesvints changed the title Allow placing a batch on a chain [11.x] Allow placing a batch on a chain Oct 10, 2023
@themsaid
Copy link
Member

Nice one.

One comment I have is that attaching the chain continuity to the batch's then callbacks means that a batch that allows failure will break the chain even though failure is allowed.

I suggest switching to a finally callback and then check if the batch is cancelled. If so, the chain is discarded, otherwise the chain continues even if the batch has failures.

@khepin
Copy link
Contributor Author

khepin commented Oct 11, 2023

@themsaid I like that. Updated accordingly, thanks for noticing that.

@taylorotwell taylorotwell changed the base branch from master to 10.x November 1, 2023 19:40
@taylorotwell taylorotwell changed the title [11.x] Allow placing a batch on a chain [10.x] Allow placing a batch on a chain Nov 1, 2023
@taylorotwell taylorotwell merged commit e5e67c4 into laravel:10.x Nov 3, 2023
20 checks passed
renovate bot added a commit to RadioRoster/backend that referenced this pull request Nov 24, 2023
[![Mend Renovate logo
banner](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [laravel/framework](https://laravel.com)
([source](https://togithub.com/laravel/framework)) | `10.30.1` ->
`10.33.0` |
[![age](https://developer.mend.io/api/mc/badges/age/packagist/laravel%2fframework/10.33.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/packagist/laravel%2fframework/10.33.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/packagist/laravel%2fframework/10.30.1/10.33.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/packagist/laravel%2fframework/10.30.1/10.33.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>laravel/framework (laravel/framework)</summary>

###
[`v10.33.0`](https://togithub.com/laravel/framework/blob/HEAD/CHANGELOG.md#v10330---2023-11-21)

[Compare
Source](https://togithub.com/laravel/framework/compare/v10.32.1...v10.33.0)

- \[10.x] Fix wrong parameter passing and add these rules to dependent
rules by [@&#8203;kayw-geek](https://togithub.com/kayw-geek) in
[laravel/framework#49008
- \[10.x] Make Validator::getValue() public by
[@&#8203;shinsenter](https://togithub.com/shinsenter) in
[laravel/framework#49007
- \[10.x] Custom messages for `Password` validation rule by
[@&#8203;rcknr](https://togithub.com/rcknr) in
[laravel/framework#48928
- \[10.x] Round milliseconds in database seeder console output runtime
by [@&#8203;SjorsO](https://togithub.com/SjorsO) in
[laravel/framework#49014
- \[10.x] Add a `Number` utility class by
[@&#8203;caendesilva](https://togithub.com/caendesilva) in
[laravel/framework#48845
- \[10.x] Fix the replace() method in DefaultService class by
[@&#8203;jonagoldman](https://togithub.com/jonagoldman) in
[laravel/framework#49022
- \[10.x] Pass the property $validator as a parameter to the $callback
Closure by [@&#8203;shinsenter](https://togithub.com/shinsenter) in
[laravel/framework#49015
- \[10.x] Fix Cache DatabaseStore::add() error occur on Postgres within
transaction by [@&#8203;xdevor](https://togithub.com/xdevor) in
[laravel/framework#49025
- \[10.x] Support asserting against chained batches by
[@&#8203;taylorotwell](https://togithub.com/taylorotwell) in
[laravel/framework#49003
- \[10.x] Prevent DB `Cache::get()` occur race condition by
[@&#8203;xdevor](https://togithub.com/xdevor) in
[laravel/framework#49031
- \[10.x] Fix notifications being counted as sent without a "shouldSend"
method by [@&#8203;joelwmale](https://togithub.com/joelwmale) in
[laravel/framework#49030
- \[10.x] Fix tests failure on Windows by
[@&#8203;hafezdivandari](https://togithub.com/hafezdivandari) in
[laravel/framework#49037
- \[10.x] Add unless conditional on validation rules by
[@&#8203;michaelnabil230](https://togithub.com/michaelnabil230) in
[laravel/framework#49048
- \[10.x] Handle string based payloads that are not JSON or form data
when creating PSR request instances by
[@&#8203;timacdonald](https://togithub.com/timacdonald) in
[laravel/framework#49047
- \[10.x] Fix directory separator CMD display on windows by
[@&#8203;imanghafoori1](https://togithub.com/imanghafoori1) in
[laravel/framework#49045
- \[10.x] Fix mapSpread doc by
[@&#8203;timacdonald](https://togithub.com/timacdonald) in
[laravel/framework#48941
- \[10.x] Tiny `Support\Collection` test fix - Unused data provider
parameter by [@&#8203;stevebauman](https://togithub.com/stevebauman) in
[laravel/framework#49053
- \[10.x] Feat: Add color_hex validation rule by
[@&#8203;nikopeikrishvili](https://togithub.com/nikopeikrishvili) in
[laravel/framework#49056
- \[10.x] Handle missing translation strings using callback by
[@&#8203;DeanWunder](https://togithub.com/DeanWunder) in
[laravel/framework#49040
- \[10.x] Add Str::transliterate to Stringable by
[@&#8203;dwightwatson](https://togithub.com/dwightwatson) in
[laravel/framework#49065
- Add Alpha Channel support to Hex validation rule by
[@&#8203;ahinkle](https://togithub.com/ahinkle) in
[laravel/framework#49069

###
[`v10.32.1`](https://togithub.com/laravel/framework/blob/HEAD/CHANGELOG.md#v10321---2023-11-14)

[Compare
Source](https://togithub.com/laravel/framework/compare/v10.32.0...v10.32.1)

- \[10.x] Add `[@pushElseIf](https://togithub.com/pushElseIf)` and
`[@pushElse](https://togithub.com/pushElse)` by
[@&#8203;jasonmccreary](https://togithub.com/jasonmccreary) in
[laravel/framework#48990

###
[`v10.32.0`](https://togithub.com/laravel/framework/blob/HEAD/CHANGELOG.md#v10320---2023-11-14)

[Compare
Source](https://togithub.com/laravel/framework/compare/v10.31.0...v10.32.0)

- Update PendingRequest.php by
[@&#8203;mattkingshott](https://togithub.com/mattkingshott) in
[laravel/framework#48939
- \[10.x] Change array_key_exists with null coalescing assignment
operator in FilesystemAdapter by
[@&#8203;miladev95](https://togithub.com/miladev95) in
[laravel/framework#48943
- \[10.x] Use container to resolve email validator class by
[@&#8203;orkhanahmadov](https://togithub.com/orkhanahmadov) in
[laravel/framework#48942
- \[10.x] Added `getGlobalMiddleware` method to HTTP Client Factory by
[@&#8203;pascalbaljet](https://togithub.com/pascalbaljet) in
[laravel/framework#48950
- \[10.x] Detect MySQL read-only mode error as a lost connection by
[@&#8203;cosmastech](https://togithub.com/cosmastech) in
[laravel/framework#48937
- \[10.x] Adds more implicit validation rules for `present` based on
other fields by
[@&#8203;diamondobama](https://togithub.com/diamondobama) in
[laravel/framework#48908
- \[10.x] Refactor set_error_handler callback to use arrow function in
`InteractsWithDeprecationHandling` by
[@&#8203;miladev95](https://togithub.com/miladev95) in
[laravel/framework#48954
- \[10.x] Test Improvements by
[@&#8203;crynobone](https://togithub.com/crynobone) in
[laravel/framework#48962
- Fix issue that prevents BladeCompiler to raise an exception when
temporal compiled blade template is not found. by
[@&#8203;juanparati](https://togithub.com/juanparati) in
[laravel/framework#48957
- \[10.x] Fix how nested transaction callbacks are handled by
[@&#8203;mateusjatenee](https://togithub.com/mateusjatenee) in
[laravel/framework#48859
- \[10.x] Fixes Batch Callbacks not triggering if job timeout while in
transaction by [@&#8203;crynobone](https://togithub.com/crynobone) in
[laravel/framework#48961
- \[10.x] expressions in migration computations fail by
[@&#8203;tpetry](https://togithub.com/tpetry) in
[laravel/framework#48976
- \[10.x] Fixes Exception: Cannot traverse an already closed generator
when running Arr::first with an empty generator and no callback by
[@&#8203;moshe-autoleadstar](https://togithub.com/moshe-autoleadstar) in
[laravel/framework#48979
- fixes issue with stderr when there was "]" character. by
[@&#8203;nikopeikrishvili](https://togithub.com/nikopeikrishvili) in
[laravel/framework#48975
- \[10.x] Fix Postgres cache store failed to put exist cache in
transaction by [@&#8203;xdevor](https://togithub.com/xdevor) in
[laravel/framework#48968

###
[`v10.31.0`](https://togithub.com/laravel/framework/blob/HEAD/CHANGELOG.md#v10310---2023-11-07)

[Compare
Source](https://togithub.com/laravel/framework/compare/v10.30.1...v10.31.0)

- \[10.x] Allow `Sleep::until()` to be passed a timestamp as a string by
[@&#8203;jameshulse](https://togithub.com/jameshulse) in
[laravel/framework#48883
- \[10.x] Fix whereHasMorph() with nullable morphs by
[@&#8203;MarkKremer](https://togithub.com/MarkKremer) in
[laravel/framework#48903
- \[10.x] Handle `class_parents` returning false in
`class_uses_recursive` by
[@&#8203;RoflCopter24](https://togithub.com/RoflCopter24) in
[laravel/framework#48902
- \[10.x] Enable default retrieval of all fragments in `fragments()` and
`fragmentsIf()` methods by [@&#8203;tabuna](https://togithub.com/tabuna)
in
[laravel/framework#48894
- \[10.x] Allow placing a batch on a chain by
[@&#8203;khepin](https://togithub.com/khepin) in
[laravel/framework#48633
- \[10.x] Dispatch 'connection failed' event in async http client
request by [@&#8203;gdebrauwer](https://togithub.com/gdebrauwer) in
[laravel/framework#48900
- authenticate method refactored to use null coalescing operator by
[@&#8203;miladev95](https://togithub.com/miladev95) in
[laravel/framework#48917
- \[10.x] Add support for Sec-Purpose header by
[@&#8203;nanos](https://togithub.com/nanos) in
[laravel/framework#48925
- \[10.x] Allow setting retain_visibility config option on Flysystem
filesystems by [@&#8203;jnoordsij](https://togithub.com/jnoordsij) in
[laravel/framework#48935
- \[10.x] Escape forward slashes when exploding wildcard rules by
[@&#8203;matt-farrugia](https://togithub.com/matt-farrugia) in
[laravel/framework#48936

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/Lapotor/RadioRoster-api).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy41OS44IiwidXBkYXRlZEluVmVyIjoiMzcuNTkuOCIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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

3 participants