Bind TaskFlow stub-task call arguments in the Go SDK runtime - #70209
Bind TaskFlow stub-task call arguments in the Go SDK runtime#70209jason810496 wants to merge 3 commits into
Conversation
3f276fe to
25e2d12
Compare
9d95d0d to
3a2cad0
Compare
A Go task could only reach an upstream task's output by hand-writing a GetXCom call against a hard-coded task id, duplicating wiring the Dag file already owns and breaking silently whenever that upstream was renamed. apache#69757 ships the Python half: a `@task.stub` TaskFlow call is captured at Dag serialization as an ordered arg-binding spec and returned by ti_run. Consuming it here lets a Go task function take the Dag's literals and upstream XComs as ordinary typed parameters. A function declares either flat positional parameters or a single struct whose fields bind by name -- kwarg-style, so an unmatched field keeps its zero value while an argument no field claims fails the task. Signature problems are caught once at registration; per-execution arity, type and spec errors fail the task before its body runs, replacing a silent reflect.Zero fill.
Several binding problems stayed quiet until they were expensive or confusing. A parameter nesting an undecodable value failed on every execution rather than once when the bundle was built. A struct carrying `arg:` tags whose single argument no tag matched was decoded whole into the struct, so a typo'd tag surfaced as a decode error naming the Go type instead of the argument that matched nothing. A value_schema or from_default of the wrong wire shape was indistinguishable from an absent one, disabling the declared-type check or turning a captured stub default into an argument the author supposedly passed. The XCom whole-value decode and the concurrent multi-pull failure path were also reachable from a Dag but exercised only in their simplest shape, and the package docs re-explained the whole binding model at three sites, burying the rules they were meant to state.
1ddfc95 to
0507c8a
Compare
The Edge Worker's execution API carries no argument spec at all, so failing a task there for an argument-count mismatch blamed the Dag author for a limit of the transport. Keeping data parameters at their Go zero values is how those tasks behaved before binding existed, and that path is in maintenance rather than gaining the spec. Registration rejected struct shapes that decode without complaint -- a struct carrying a callback alongside its data never needed the callback filled -- and because registering a task panics, one such signature took its whole bundle down at startup rather than the single task. Adding a defaulted parameter to a stub is backwards compatible in Python, and has to stay so for the Go functions already bound to that stub: the captured default reaches the wire but needs no Go parameter to receive it. Untagged fields matched a Go field name verbatim, which no idiomatic snake_case stub parameter can produce, so tags were mandatory in practice and a mismatch quietly fell back to decoding the argument whole. Folding case and underscores makes the untagged form usable, and embedded structs now contribute their fields the way encoding/json has all along. A type that decodes itself from JSON also passed registration only to be rejected at run time by a schema check judging it on its Go kind.
0507c8a to
be8f718
Compare
There was a problem hiding this comment.
Should we add a hook to ensure this file is up-to-date? (Or at least not incorrectly regenerated.)
There was a problem hiding this comment.
IIRC, the last consensus we had for this is to bump the generated file manually instead of having fully automate static check.
Since the one who change the Task SDK schema might not able to change the Lang SDK side generated schema.
There was a problem hiding this comment.
(Or at least not incorrectly regenerated.)
Make sense for this part, the best way I can come up with is vendoring the the supervisor schema JSON. But this introduce another question: How can we ensure the vendored supervisor schema JSON is correct?
We already had e2e test to coverage all the features in Go SDK, so I think it's fine not to have a static check to guard the auto generated schema. If it's a malformed schema, it can't even pass the e2e test.
Why
#69757 ships the Python-side contract: a
@task.stubTaskFlow call is serialized as an ordered arg-binding spec and returned byti_runasTIRunContext.arg_bindings. This PR makes the Go SDK actually consume it -- Go task functions receive the Dag file's literals and upstream XComs as typed parameters instead of hand-writingGetXComcalls with hard-coded upstream task ids:Supported TaskFlow syntax
The example bundle's
taskflow_binding_dag(go-sdk/dags/go_examples.py+go-sdk/example/bundle/taskflowbinding/) exercises the full surface end to end:On the Go side a task declares either flat positional data parameters or a single struct whose fields bind by name (mixing the two shapes is rejected at registration as too ambiguous):
Binding semantics, mirroring positional vs keyword calls:
fill every data parameter, with
Nonedecoded into nil-capable types.arg:tag. Unmatched fields keep their zero value,from_defaultentries may go unclaimed, and explicitly passed unclaimed arguments fail.not use this fallback.
schema mismatches, and strict struct decode errors are reported instead of silently zero-filling.
How
New
pkg/bindingpackage:Analyzeclassifies parameters once at registration (injectables vs JSON-decodable data parameters, or the single name-bound struct), recursing through nested types so an undecodable one fails when the bundle is built rather than on every execution.Resolvebinds the wire spec onto them. The wire union surfaces as a sealed sum type (binding.XComArg/binding.LiteralArg) whose variants andDataTypevocabulary are defined in terms of the generatedgenmodelsschema types, so the runtime types cannot drift from the wire model.Rejected Alternative
xcom:/xcom-key:struct tags were considered and dropped. A task that needs an extra XCom still asks the injected client explicitly.Was generative AI tooling used to co-author this PR?