ashb commented on code in PR #55443:
URL: https://github.com/apache/airflow/pull/55443#discussion_r2334885655
##########
airflow-core/src/airflow/utils/serve_logs/log_server.py:
##########
@@ -157,4 +157,17 @@ def create_app():
return fastapi_app
-app = create_app()
+app = None
+
+
+def get_app():
+ """Get or create the FastAPI app instance."""
+ global app
+ if app is None:
+ app = create_app()
+ return app
+
+
+def create_app_for_uvicorn():
+ """Create the FastAPI app for uvicorn."""
+ return get_app()
Review Comment:
Why do we need a separate function here?
##########
airflow-core/src/airflow/utils/serve_logs/core.py:
##########
@@ -53,12 +53,16 @@ def serve_logs(port=None):
logger.info("Starting log server on %s", serve_log_uri)
- # Use uvicorn directly for ASGI applications
- uvicorn.run("airflow.utils.serve_logs.log_server:app", host=host,
port=port, workers=2, log_level="info")
- # Note: if we want to use more than 1 workers, we **can't** use the
instance of FastAPI directly
- # This is way we split the instantiation of log server to a separate module
- #
- #
https://github.com/encode/uvicorn/blob/374bb6764e8d7f34abab0746857db5e3d68ecfdd/docs/deployment/index.md?plain=1#L50-L63
+ # Use uvicorn with factory pattern for lazy app creation
+ # This ensures proper initialization order: settings.initialize() →
configure_logging() → create_app()
+ uvicorn.run(
+ "airflow.utils.serve_logs.log_server:create_app_for_uvicorn",
Review Comment:
```suggestion
"airflow.utils.serve_logs.log_server:get_app",
```
World work too, no?
##########
airflow-core/src/airflow/utils/serve_logs/core.py:
##########
@@ -53,12 +53,16 @@ def serve_logs(port=None):
logger.info("Starting log server on %s", serve_log_uri)
- # Use uvicorn directly for ASGI applications
- uvicorn.run("airflow.utils.serve_logs.log_server:app", host=host,
port=port, workers=2, log_level="info")
- # Note: if we want to use more than 1 workers, we **can't** use the
instance of FastAPI directly
- # This is way we split the instantiation of log server to a separate module
- #
- #
https://github.com/encode/uvicorn/blob/374bb6764e8d7f34abab0746857db5e3d68ecfdd/docs/deployment/index.md?plain=1#L50-L63
+ # Use uvicorn with factory pattern for lazy app creation
+ # This ensures proper initialization order: settings.initialize() →
configure_logging() → create_app()
Review Comment:
I'm not sure this actually changes anything, since in importing this module,
starlette will import airflow first, which will init the logging - because when
using multiple workers it loads the app in a freshly spawned python process
with nothing loaded
--
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]
