ashb commented on code in PR #61630:
URL: https://github.com/apache/airflow/pull/61630#discussion_r2923974639
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -820,7 +820,7 @@ def parse(what: StartupDetails, log: Logger) ->
RuntimeTaskInstance:
# deeply nested execution stack.
# - By defining `SUPERVISOR_COMMS` as a global, it ensures that this
communication mechanism is readily
# accessible wherever needed during task execution without modifying every
layer of the call stack.
-SUPERVISOR_COMMS: CommsDecoder[ToTask, ToSupervisor]
+SUPERVISOR_COMMS: CommsDecoder[ToTask, ToSupervisor] = None # type:
ignore[assignment]
Review Comment:
Especially don't set it to none but then type ignore it. If it can be none
we'd need to add not None checks everywhere we access it
##########
task-sdk/src/airflow/sdk/execution_time/supervisor.py:
##########
@@ -1747,28 +1747,19 @@ def set_supervisor_comms(temp_comms):
by injecting a test Comms implementation (e.g. `InProcessSupervisorComms`)
in place of the real inter-process communication layer.
- Some parts of the code (e.g. models.Variable.get) check for the presence
- of `task_runner.SUPERVISOR_COMMS` to determine if the code is running in a
Task SDK execution context.
- This override ensures those code paths behave correctly during in-process
tests.
+ Some parts of the code (e.g. models.Variable.get) check that
+ `task_runner.SUPERVISOR_COMMS` is not None to determine if the code is
running in a Task SDK
Review Comment:
Please leave this as attribute not set when it's not initalized and instead
of not None checks add hasattr checks to the few places.
--
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]
