[I] Share AIRFLOW_VAR_* env-override helper between HTTP and coordinator Go SDK clients [airflow]

["jason810496 (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@airflow.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] Share AIRFLOW_VAR_* env-override helper between HTTP and coordinator Go SDK clients [airflow]

["shaealh (via GitHub)" <gi...@apache.org>]

shaealh commented on issue #67598:
URL: https://github.com/apache/airflow/issues/67598#issuecomment-4999726229

   @jason810496 I [opened a PR](https://github.com/apache/airflow/pull/69899) to address this 


-- 
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: [I] Share AIRFLOW_VAR_* env-override helper between HTTP and coordinator Go SDK clients [airflow]

["jason810496 (via GitHub)" <gi...@apache.org>]

jason810496 closed issue #67598: Share AIRFLOW_VAR_* env-override helper between HTTP and coordinator Go SDK clients
URL: https://github.com/apache/airflow/issues/67598


-- 
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: [I] Share AIRFLOW_VAR_* env-override helper between HTTP and coordinator Go SDK clients [airflow]

["jason810496 (via GitHub)" <gi...@apache.org>]

jason810496 commented on issue #67598:
URL: https://github.com/apache/airflow/issues/67598#issuecomment-5356513512

   Close this issue as https://github.com/apache/airflow/pull/71874 was merged. We don't need to consolidate the client anymore.


-- 
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