feat(proxy_server.py): support graceful shutdowns #9282
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| ### LOAD CONFIG ### | ||
| worker_config: Optional[Union[str, dict]] = get_secret("WORKER_CONFIG") # type: ignore | ||
| env_config_yaml: Optional[str] = get_secret_str("CONFIG_FILE_PATH") | ||
| verbose_proxy_logger.debug("worker_config: %s", worker_config) |
Check failure
Code scanning / CodeQL
Clear-text logging of sensitive information High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 10 months ago
To fix the problem, we should avoid logging sensitive information directly. Instead, we can log a message indicating that the configuration was loaded without revealing its contents. This way, we maintain the functionality of logging important events without exposing sensitive data.
- Replace the line that logs
worker_configwith a more generic log message. - Ensure that no sensitive information is logged in clear text.
| @@ -516,3 +516,3 @@ | ||
| env_config_yaml: Optional[str] = get_secret_str("CONFIG_FILE_PATH") | ||
| verbose_proxy_logger.debug("worker_config: %s", worker_config) | ||
| verbose_proxy_logger.debug("worker_config loaded successfully") | ||
| # check if it's a valid file path |
litellm/proxy/proxy_server.py
Outdated
| if isinstance(worker_config, dict): | ||
| await initialize(**worker_config) | ||
| # Register signal handlers | ||
| signal.signal(signal.SIGTERM, handle_shutdown) |
There was a problem hiding this comment.
I wonder how this approach works w/ uvicorn and gunicorn's way of handling graceful timeouts.
There was a problem hiding this comment.
is there a different signal they issue?
There was a problem hiding this comment.
Looks like both gunicorn and uvicorn issue a SIGTERM, so this should still work
litellm/proxy/proxy_server.py
Outdated
| verbose_proxy_logger.info( | ||
| f"Graceful shutdown initiated. Waiting up to {shutdown_timeout}s for requests to complete..." | ||
| ) | ||
| await asyncio.sleep(shutdown_timeout) |
There was a problem hiding this comment.
Could we condition this on minimum of [all server connections terminating, shutdown_timeout]? From my understanding, this logic will result in all instances taking the full shutdown_timeout which is not as ideal
…ensure tests not impacted
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
Title
Relevant issues
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit)[https://docs.litellm.ai/docs/extras/contributing_code]Type
🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test
Changes