henry3260 commented on code in PR #65587:
URL: https://github.com/apache/airflow/pull/65587#discussion_r3469928954
##########
task-sdk/src/airflow/sdk/log.py:
##########
@@ -265,6 +266,15 @@ def upload_to_remote(logger: FilteringBoundLogger, ti:
RuntimeTI | None = None):
)
+def _is_supervisor_comms_ready_for_mask_secret(comms: Any) -> bool:
+ """Return whether mask notifications can be sent without using stale
virtualenv comms."""
+ if not comms:
+ return False
+ if os.environ.get("PYTHON_OPERATORS_VIRTUAL_ENV_MODE"):
+ return getattr(comms, "socket", None) is not None
+ return True
Review Comment:
> May I ask why do we need this change? IIUC, if `SUPERVISOR_COMMS` existed,
then the `SUPERVISOR_COMMS.socket` should be existed as well. And why only the
`PYTHON_OPERATORS_VIRTUAL_ENV_MODE` case need to check the additional `socket`?
yes, you are right. Before I consider the edge case virtualenv child will
inherit old `SUPERVISOR_COMMS `Python object, but i think i am wrong i think
child will build a new one
--
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]
