jason810496 opened a new issue, #67598:
URL: https://github.com/apache/airflow/issues/67598
### Background
The Go SDK has two independent implementations of the same Python-parity
rule that an environment variable named ``AIRFLOW_VAR_<UPPER(key)>``
short-circuits ``GetVariable`` and is returned in preference to the
supervisor-stored value (matching ``airflow.models.variable.Variable.get``).
The HTTP-backed client uses an unexported helper:
```go
// go-sdk/sdk/client.go
func variableFromEnv(key string) (string, bool) {
return os.LookupEnv(VariableEnvPrefix + strings.ToUpper(key))
}
func (*client) GetVariable(ctx context.Context, key string) (string, error) {
// TODO: Let the lookup priority be configurable like it is in Python SDK
if env, ok := variableFromEnv(key); ok {
return env, nil
}
...
}
```
The coordinator-mode client inlines the same logic and carries a TODO
calling out the duplication:
```go
// go-sdk/pkg/execution/client.go
func (c *CoordinatorClient) GetVariable(ctx context.Context, key string)
(string, error) {
// TODO: this duplicates variableFromEnv in sdk/client.go. The env-first
// precedence is part of the SDK contract, so both clients should share a
// single helper (e.g. an exported sdk.VariableFromEnv) instead of two
// independent copies that can drift.
if env, ok := os.LookupEnv(sdk.VariableEnvPrefix +
strings.ToUpper(key)); ok {
return env, nil
}
...
}
```
The env-first lookup order is part of the public Go SDK contract; two
independently maintained copies can silently drift, particularly if the lookup
precedence is made configurable later (as the HTTP-side TODO notes).
### What needs to happen
1. Promote ``variableFromEnv`` to an exported helper on the ``sdk`` package
— e.g. ``sdk.VariableFromEnv(key string) (string, bool)`` — so the env-override
logic has a single canonical implementation.
2. Update ``go-sdk/sdk/client.go`` to call the exported helper.
3. Update ``go-sdk/pkg/execution/client.go``
``CoordinatorClient.GetVariable`` to call the same exported helper and drop the
inlined ``os.LookupEnv`` block.
4. Remove the duplication TODO in ``pkg/execution/client.go``.
### Acceptance criteria
- Only one implementation of the ``AIRFLOW_VAR_*`` env-override lookup
exists in the Go SDK source tree.
- Both ``sdk.client`` and ``execution.CoordinatorClient`` call the shared
helper.
- Existing tests (``TestCoordinatorClientGetVariableEnvOverride`` and any
HTTP-side equivalent) continue to pass without modification, or are updated to
exercise the shared helper directly if duplication of the test coverage no
longer makes sense.
### Dependency
Wait until #67317 merges — the coordinator-mode duplication lands in that
PR, so the dedup work can only start once both copies exist on ``main``.
### Context
- Originating review: self-review thread on #67317 (coordinator comms +
client PR).
- HTTP-side helper: ``go-sdk/sdk/client.go`` (``variableFromEnv``).
- Coordinator-side duplicate (lands with #67317):
``go-sdk/pkg/execution/client.go`` ``CoordinatorClient.GetVariable``.
- Related project board: https://github.com/orgs/apache/projects/499/views/1
---
Drafted-by: Claude Code (Opus 4.7) (no human review before posting)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
