You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "jason810496 (via GitHub)" <gi...@apache.org> on 2026/06/10 17:14:09 UTC
[PR] Go-SDK: Inject task runtime context as sdk.TIRunContext parameter [airflow]
jason810496 opened a new pull request, #68349:
URL: https://github.com/apache/airflow/pull/68349
- related: https://github.com/apache/airflow/pull/68271
- docs side: https://github.com/apache/airflow/pull/68319
## Why
As mentioned by @ashb and @amoghrajesh in https://github.com/apache/airflow/pull/68319 that the previous `runtimeContext := sdk.CurrentContext(ctx)` was unclear and ambiguous.
## What
Change the user interface as `func extract(ctx sdk.TIRunContext, client sdk.Client, log *slog.Logger) (any, error) {` but the injection mechanism still able to recognize the Edge Worker compatible `func extract(ctx context.Context, client sdk.Client, log *slog.Logger) (any, error) {`-style function interface.
---
##### Was generative AI tooling used to co-author this PR?
- [x] Yes, with help of Claude Code (Opus 4.8) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Go-SDK: Inject task runtime context as sdk.TIRunContext parameter [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #68349:
URL: https://github.com/apache/airflow/pull/68349#discussion_r3393882316
##########
go-sdk/sdk/context.go:
##########
@@ -20,29 +20,48 @@ package sdk
import (
"context"
"time"
-
- "github.com/apache/airflow/go-sdk/pkg/sdkcontext"
)
-// RuntimeContext carries the identifiers and scheduling timestamps of the task
-// instance that is currently executing, along with the Dag run it belongs to.
-// It is the Go equivalent of the execution context the Python and Java SDKs
-// expose to task authors.
+// TIRunContext is the execution context handed to a task. It embeds the
+// standard context.Context (cancellation, deadline, request-scoped values) and
+// additionally carries the identifiers and scheduling timestamps of the task
+// instance that is executing, along with the Dag run it belongs to. It is the
+// Go equivalent of the execution context the Python and Java SDKs expose to
+// task authors.
//
-// Retrieve it inside a task function with CurrentContext:
+// The runtime injects it into a task function by parameter type, so declare it
+// as the task's context argument and read the fields directly:
//
-// func myTask(ctx context.Context, log *slog.Logger) error {
-// rc := sdk.CurrentContext(ctx)
-// log.Info("running", "task_id", rc.TI.TaskID, "run_id", rc.DagRun.RunID)
+// func myTask(ctx sdk.TIRunContext, log *slog.Logger) error {
+// log.Info("running", "task_id", ctx.TI.TaskID, "run_id", ctx.DagRun.RunID)
// return nil
// }
-type RuntimeContext struct {
+//
+// Because it embeds context.Context it is itself a context.Context: pass it
+// straight to client calls, select on ctx.Done(), or hand it to downstream
+// helpers that take a context.Context.
+//
+// Embedding a context.Context in a struct is normally discouraged, but it is a
+// deliberate exception here: the runtime needs a concrete type to bind by
+// parameter, and the value lives only for the duration of a single task
+// execution. The runtime always sets the embedded Context before invoking the
+// task. When constructing one yourself (for example in a unit test) set the
+// Context field, otherwise the context.Context methods dereference a nil
+// interface and panic:
+//
+// ctx := sdk.TIRunContext{Context: context.Background()}
+type TIRunContext struct {
+ context.Context
Review Comment:
Duplicate our offline discussion here for further reviewer reference.
I agree with Go programming level perspective we should separate (actually the first version I draft use the separate `ctx context.Context, tiRunCtx sdk.TIRunContext` ).
However, for the SDK developer experience perspective, would it be acceptable and more clear to have `ctx sdk.TIRunContext` as user interface?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Go-SDK: Inject task runtime context as sdk.TIRunContext parameter [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #68349:
URL: https://github.com/apache/airflow/pull/68349#discussion_r3395023517
##########
go-sdk/bundle/bundlev1/task.go:
##########
@@ -61,7 +61,22 @@ func (f *taskFunction) Execute(ctx context.Context, logger *slog.Logger) error {
in := fnType.In(i)
switch {
+ case isTIRunContext(in):
+ // sdk.TIRunContext embeds context.Context, so it also satisfies
Review Comment:
Outdated?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Go-SDK: Inject task runtime context as sdk.TIRunContext parameter [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #68349:
URL: https://github.com/apache/airflow/pull/68349#discussion_r3393706304
##########
go-sdk/sdk/context.go:
##########
@@ -20,29 +20,48 @@ package sdk
import (
"context"
"time"
-
- "github.com/apache/airflow/go-sdk/pkg/sdkcontext"
)
-// RuntimeContext carries the identifiers and scheduling timestamps of the task
-// instance that is currently executing, along with the Dag run it belongs to.
-// It is the Go equivalent of the execution context the Python and Java SDKs
-// expose to task authors.
+// TIRunContext is the execution context handed to a task. It embeds the
+// standard context.Context (cancellation, deadline, request-scoped values) and
+// additionally carries the identifiers and scheduling timestamps of the task
+// instance that is executing, along with the Dag run it belongs to. It is the
+// Go equivalent of the execution context the Python and Java SDKs expose to
+// task authors.
//
-// Retrieve it inside a task function with CurrentContext:
+// The runtime injects it into a task function by parameter type, so declare it
+// as the task's context argument and read the fields directly:
//
-// func myTask(ctx context.Context, log *slog.Logger) error {
-// rc := sdk.CurrentContext(ctx)
-// log.Info("running", "task_id", rc.TI.TaskID, "run_id", rc.DagRun.RunID)
+// func myTask(ctx sdk.TIRunContext, log *slog.Logger) error {
+// log.Info("running", "task_id", ctx.TI.TaskID, "run_id", ctx.DagRun.RunID)
// return nil
// }
-type RuntimeContext struct {
+//
+// Because it embeds context.Context it is itself a context.Context: pass it
+// straight to client calls, select on ctx.Done(), or hand it to downstream
+// helpers that take a context.Context.
+//
+// Embedding a context.Context in a struct is normally discouraged, but it is a
+// deliberate exception here: the runtime needs a concrete type to bind by
+// parameter, and the value lives only for the duration of a single task
+// execution. The runtime always sets the embedded Context before invoking the
+// task. When constructing one yourself (for example in a unit test) set the
+// Context field, otherwise the context.Context methods dereference a nil
+// interface and panic:
+//
+// ctx := sdk.TIRunContext{Context: context.Background()}
+type TIRunContext struct {
+ context.Context
Review Comment:
One thing worth discussing: embedding `context.Context` in a struct is a Go anti pattern the docs explicitly advises against (https://pkg.go.dev/context#hdr-Contexts_and_structs). This is cos because structs tend to outlive the request they were created for, so the stored context's cancellation/deadline silently carries over into unintended reuse.
The alternative that avoids this entirely: keep `context.Context` as `context.Context` and inject the `TI/DagRun` metadata as a separate plain struct maybe:
```go
func myTask(ctx context.Context, run sdk.RunInfo, log *slog.Logger) error {
log.Info("running", "task_id", run.TI.TaskID)
client.GetVariable(ctx, "my-var")
}
```
worth considering before the API stabilises.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Go-SDK: Inject task runtime context as sdk.TIRunContext parameter [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #68349:
URL: https://github.com/apache/airflow/pull/68349#discussion_r3393882316
##########
go-sdk/sdk/context.go:
##########
@@ -20,29 +20,48 @@ package sdk
import (
"context"
"time"
-
- "github.com/apache/airflow/go-sdk/pkg/sdkcontext"
)
-// RuntimeContext carries the identifiers and scheduling timestamps of the task
-// instance that is currently executing, along with the Dag run it belongs to.
-// It is the Go equivalent of the execution context the Python and Java SDKs
-// expose to task authors.
+// TIRunContext is the execution context handed to a task. It embeds the
+// standard context.Context (cancellation, deadline, request-scoped values) and
+// additionally carries the identifiers and scheduling timestamps of the task
+// instance that is executing, along with the Dag run it belongs to. It is the
+// Go equivalent of the execution context the Python and Java SDKs expose to
+// task authors.
//
-// Retrieve it inside a task function with CurrentContext:
+// The runtime injects it into a task function by parameter type, so declare it
+// as the task's context argument and read the fields directly:
//
-// func myTask(ctx context.Context, log *slog.Logger) error {
-// rc := sdk.CurrentContext(ctx)
-// log.Info("running", "task_id", rc.TI.TaskID, "run_id", rc.DagRun.RunID)
+// func myTask(ctx sdk.TIRunContext, log *slog.Logger) error {
+// log.Info("running", "task_id", ctx.TI.TaskID, "run_id", ctx.DagRun.RunID)
// return nil
// }
-type RuntimeContext struct {
+//
+// Because it embeds context.Context it is itself a context.Context: pass it
+// straight to client calls, select on ctx.Done(), or hand it to downstream
+// helpers that take a context.Context.
+//
+// Embedding a context.Context in a struct is normally discouraged, but it is a
+// deliberate exception here: the runtime needs a concrete type to bind by
+// parameter, and the value lives only for the duration of a single task
+// execution. The runtime always sets the embedded Context before invoking the
+// task. When constructing one yourself (for example in a unit test) set the
+// Context field, otherwise the context.Context methods dereference a nil
+// interface and panic:
+//
+// ctx := sdk.TIRunContext{Context: context.Background()}
+type TIRunContext struct {
+ context.Context
Review Comment:
Duplicate our offline discussion here for further reviewer reference.
---
I agree with Go programming level perspective we should separate (actually the first version I draft use the separate `ctx context.Context, tiRunCtx sdk.TIRunContext` ).
However, for the SDK developer experience perspective, would it be acceptable and more clear to have `ctx sdk.TIRunContext` as user interface?
---
UPDATE:
We will go with interface approach instead of packing context in struct.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Go-SDK: Inject task runtime context as sdk.TIRunContext parameter [airflow]
Posted by "amoghrajesh (via GitHub)" <gi...@apache.org>.
amoghrajesh commented on code in PR #68349:
URL: https://github.com/apache/airflow/pull/68349#discussion_r3395020996
##########
go-sdk/bundle/bundlev1/task.go:
##########
@@ -148,9 +163,10 @@ func isValidResultType(inType reflect.Type) bool {
}
var (
- errorType = reflect.TypeFor[error]()
- contextType = reflect.TypeFor[context.Context]()
- slogLoggerType = reflect.TypeFor[*slog.Logger]()
+ errorType = reflect.TypeFor[error]()
+ contextType = reflect.TypeFor[context.Context]()
+ tiRunContextType = reflect.TypeFor[sdk.TIRunContext]()
Review Comment:
Yeah 😄
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Go-SDK: Inject task runtime context as sdk.TIRunContext parameter [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #68349:
URL: https://github.com/apache/airflow/pull/68349#discussion_r3394969896
##########
go-sdk/bundle/bundlev1/task.go:
##########
@@ -148,9 +163,10 @@ func isValidResultType(inType reflect.Type) bool {
}
var (
- errorType = reflect.TypeFor[error]()
- contextType = reflect.TypeFor[context.Context]()
- slogLoggerType = reflect.TypeFor[*slog.Logger]()
+ errorType = reflect.TypeFor[error]()
+ contextType = reflect.TypeFor[context.Context]()
+ tiRunContextType = reflect.TypeFor[sdk.TIRunContext]()
Review Comment:
I wonder if this should be a pointer.
`context.Context` isn't as that is an Interface, but TIRunContext is an actual struct, so I think a pointer is more apt to pass? Not sure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Go-SDK: Inject task runtime context as sdk.TIRunContext parameter [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #68349:
URL: https://github.com/apache/airflow/pull/68349#discussion_r3395383680
##########
go-sdk/bundle/bundlev1/task.go:
##########
@@ -61,7 +61,22 @@ func (f *taskFunction) Execute(ctx context.Context, logger *slog.Logger) error {
in := fnType.In(i)
switch {
+ case isTIRunContext(in):
+ // sdk.TIRunContext embeds context.Context, so it also satisfies
Review Comment:
The sentence was written "when TIRunContext was a struct", but still keep it as the interface embeds context.Context too (interface embedding), which is the correct Go term for both forms.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Go-SDK: Inject task runtime context as sdk.TIRunContext parameter [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 commented on code in PR #68349:
URL: https://github.com/apache/airflow/pull/68349#discussion_r3393882316
##########
go-sdk/sdk/context.go:
##########
@@ -20,29 +20,48 @@ package sdk
import (
"context"
"time"
-
- "github.com/apache/airflow/go-sdk/pkg/sdkcontext"
)
-// RuntimeContext carries the identifiers and scheduling timestamps of the task
-// instance that is currently executing, along with the Dag run it belongs to.
-// It is the Go equivalent of the execution context the Python and Java SDKs
-// expose to task authors.
+// TIRunContext is the execution context handed to a task. It embeds the
+// standard context.Context (cancellation, deadline, request-scoped values) and
+// additionally carries the identifiers and scheduling timestamps of the task
+// instance that is executing, along with the Dag run it belongs to. It is the
+// Go equivalent of the execution context the Python and Java SDKs expose to
+// task authors.
//
-// Retrieve it inside a task function with CurrentContext:
+// The runtime injects it into a task function by parameter type, so declare it
+// as the task's context argument and read the fields directly:
//
-// func myTask(ctx context.Context, log *slog.Logger) error {
-// rc := sdk.CurrentContext(ctx)
-// log.Info("running", "task_id", rc.TI.TaskID, "run_id", rc.DagRun.RunID)
+// func myTask(ctx sdk.TIRunContext, log *slog.Logger) error {
+// log.Info("running", "task_id", ctx.TI.TaskID, "run_id", ctx.DagRun.RunID)
// return nil
// }
-type RuntimeContext struct {
+//
+// Because it embeds context.Context it is itself a context.Context: pass it
+// straight to client calls, select on ctx.Done(), or hand it to downstream
+// helpers that take a context.Context.
+//
+// Embedding a context.Context in a struct is normally discouraged, but it is a
+// deliberate exception here: the runtime needs a concrete type to bind by
+// parameter, and the value lives only for the duration of a single task
+// execution. The runtime always sets the embedded Context before invoking the
+// task. When constructing one yourself (for example in a unit test) set the
+// Context field, otherwise the context.Context methods dereference a nil
+// interface and panic:
+//
+// ctx := sdk.TIRunContext{Context: context.Background()}
+type TIRunContext struct {
+ context.Context
Review Comment:
Duplicate our offline discussion here for further reviewer reference.
---
I agree with Go programming level perspective we should separate (actually the first version I draft use the separate `ctx context.Context, tiRunCtx sdk.TIRunContext` ).
However, for the SDK developer experience perspective, would it be acceptable and more clear to have `ctx sdk.TIRunContext` as user interface?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Go-SDK: Inject task runtime context as sdk.TIRunContext parameter [airflow]
Posted by "ashb (via GitHub)" <gi...@apache.org>.
ashb commented on code in PR #68349:
URL: https://github.com/apache/airflow/pull/68349#discussion_r3394975128
##########
go-sdk/bundle/bundlev1/task.go:
##########
@@ -148,9 +163,10 @@ func isValidResultType(inType reflect.Type) bool {
}
var (
- errorType = reflect.TypeFor[error]()
- contextType = reflect.TypeFor[context.Context]()
- slogLoggerType = reflect.TypeFor[*slog.Logger]()
+ errorType = reflect.TypeFor[error]()
+ contextType = reflect.TypeFor[context.Context]()
+ tiRunContextType = reflect.TypeFor[sdk.TIRunContext]()
Review Comment:
Oh my mistake, this is an interface. Ignore me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Go-SDK: Inject task runtime context as sdk.TIRunContext parameter [airflow]
Posted by "jason810496 (via GitHub)" <gi...@apache.org>.
jason810496 merged PR #68349:
URL: https://github.com/apache/airflow/pull/68349
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org