feat: overload run and runNoWait to accept multiple options #3421
feat: overload run and runNoWait to accept multiple options #3421Spid3rrr wants to merge 2 commits intohatchet-dev:mainfrom
Conversation
|
Someone is attempting to deploy a commit to the Hatchet Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
This PR extends the TypeScript SDK workflow/task triggering APIs to support passing per-item RunOpts[] when input is an array, while preserving the existing “shared options” behavior and adding validation + tests.
Changes:
- Add overloads for
run()/runNoWait()(and task wrappers) to acceptRunOpts[]for array inputs. - Implement per-item option application and validate
options.length === input.lengthfor array inputs. - Add unit tests intended to cover shared options, per-item options, and mismatch errors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
sdks/typescript/src/v1/declaration.ts |
Adds option-array overloads and logic for per-item run options in array triggering paths. |
sdks/typescript/src/v1/declaration.spec.ts |
Adds tests for shared/per-item options and length mismatch validation. |
| parentRunContextManager.incrementChildIndex(Array.isArray(input) ? input.length : 1); | ||
|
|
There was a problem hiding this comment.
parentRunContextManager.incrementChildIndex(...) is called twice in runNoWait (once near the top and again after throwIfAborted). This double-increments the stored childIndex and can cause gaps/incorrect ordering for subsequent child spawns; remove one of the calls (typically keep the one after the abort precheck) and ensure the childIndex used for each spawned run is computed from the intended base.
| parentRunContextManager.incrementChildIndex(Array.isArray(input) ? input.length : 1); |
There was a problem hiding this comment.
This isn't related to my PR so I didn't test this behavior specifically. Can include the fix if preferred.
| const refs = Array.isArray(options) | ||
| ? await this.runNoWait(input, options, _standaloneTaskName) | ||
| : await this.runNoWait(input, options, _standaloneTaskName); |
There was a problem hiding this comment.
This ternary is redundant: both branches call this.runNoWait(input, options, _standaloneTaskName). It can be simplified to a single call for clarity.
| const refs = Array.isArray(options) | |
| ? await this.runNoWait(input, options, _standaloneTaskName) | |
| : await this.runNoWait(input, options, _standaloneTaskName); | |
| const refs = await this.runNoWait(input, options, _standaloneTaskName); |
There was a problem hiding this comment.
This ternary looks redundant but its purpose is to tell TS which version of runNoWait is being used. It otherwise would present an overload error. This is the 'cleanest' way I found to keep it tidy.
|
📝 Documentation updates detected! New suggestion: Document per-item run options for TypeScript SDK bulk runs Tip: Planning a big docs refactor? Use Deep Analysis to get help with the heavy lifting 🔍 |
gregfurman
left a comment
There was a problem hiding this comment.
Thanks for the contribution!
I think we should consider whether we want to be further overloading the workflow methods versus introducing specific multi-run methods -- similar to how the python and Go SDKs work.
There was a problem hiding this comment.
Can you possibly add an example and e2e test for these multi option runs in sdks/typescript/src/v1/examples?
| input: I[], | ||
| options?: RunOpts[], |
There was a problem hiding this comment.
I'm concerned that this approach deviates too much from how the other SDKs expose similar multi-run functionality. What do you think of instead exposing a runMany method similar to the Go client?
Lines 721 to 723 in 45f1203
Then we could wrap the admin client's runWorkflows
hatchet/sdks/typescript/src/v1/client/admin.ts
Lines 187 to 196 in 7e0356a
With some kind of unified RunManyOpt (which is composed by input and RunOpt)
i.e
type RunManyOpt<I> = {
input: I;
opts?: RunOpts;
};async runMany(runs: RunManyOpt<I>[]): Promise<O[]>;Then usage could look like:
// ...
const workflow = hatchet.workflow({ name: 'simple-multi-workflow' });
workflow.task({
name: 'step1',
fn: async (input: { message: string }) => ({ result: input.message }),
});
const results = await workflow.runMany([
{ input: { message: 'hello' } },
{ input: { message: 'world' } },
]);
// ...Wdyt?
There was a problem hiding this comment.
I like this approach. I wasn't aware the other sdks were exposing a different style for scheduling rus. I'll update the PR with the new approaches once done.
There was a problem hiding this comment.
@gregfurman Would we want to keep backwards compatibility (no breaking changes) for the existing runNoWait? Meaning that muliple runs can be scheduled using both runMany and run?
There was a problem hiding this comment.
Forcing the use of runMany for all bulk operations would break every run called with multiple inputs.
The alternative would be leaving it as is and simply adding the runMany method to the side (and updating the docs to encourage the use of runMany).
There was a problem hiding this comment.
The alternative would be leaving it as is and simply adding the runMany method to the side (and updating the docs to encourage the use of runMany).
Yeah this is what I was thinking. Similar to the other SDKs, think that keeping both around is best.
(cc @mrkaye97)
Description
This PR updates workflow run option handling for array inputs and adds targeted tests to verify the behavior.
When running workflows with array input:
Fixes #3398
Type of change
What's Changed
RunOpts[]inrunNoWaitwhen input is an array.