jason810496 commented on code in PR #65587:
URL: https://github.com/apache/airflow/pull/65587#discussion_r3411004591
##########
airflow-core/src/airflow/api_fastapi/execution_api/app.py:
##########
@@ -391,14 +406,20 @@ async def always_allow(request: Request):
return self._app
+ @cached_property
+ def asgi_app(self) -> ASGIApp:
+ if self.request_scoped_server_context:
+ return _RequestScopedServerContextApp(self.app)
+ return self.app
+
@cached_property
def transport(self) -> httpx.WSGITransport:
import asyncio
import httpx
from a2wsgi import ASGIMiddleware
- middleware = ASGIMiddleware(self.app)
+ middleware = ASGIMiddleware(cast("A2WSGIApp", self.asgi_app))
Review Comment:
Then we can revert this line change.
##########
airflow-core/src/airflow/process_context.py:
##########
Review Comment:
I'm on the other side to keep this as-is so the lifecycle of
`_PROCESS_CONTEXT_OVERRIDE` will be more clear but no strong opinion for this.
##########
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`?
##########
airflow-core/src/airflow/api_fastapi/execution_api/app.py:
##########
@@ -391,14 +406,20 @@ async def always_allow(request: Request):
return self._app
+ @cached_property
+ def asgi_app(self) -> ASGIApp:
+ if self.request_scoped_server_context:
+ return _RequestScopedServerContextApp(self.app)
+ return self.app
Review Comment:
It seems no need to introduce another `cached_property`, making change at
end of existing `app` `cached_property` would be sufficient.
```suggestion
if self.request_scoped_server_context:
return _RequestScopedServerContextApp(self._app)
return self._app
```
--
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]
