Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enable sensitive log mode #415

Merged
merged 8 commits into from Jan 4, 2024
Merged

enable sensitive log mode #415

merged 8 commits into from Jan 4, 2024

Conversation

song-william
Copy link
Collaborator

@song-william song-william commented Dec 31, 2023

Pull Request Summary

Enable sensitive log mode for use cases that want avoid logging user-specific data (e.g prompts and responses)

Test Plan and Usage Guide

Deploy in training and confirmed in datadog that logs no longer show up in vllm or logged by our gateway.

@@ -156,6 +156,8 @@ def test_completion_sync_endpoint_not_found_returns_404(
assert response_1.status_code == 404


# When enabling this test, other tests fail with "RunTumeError got Future <Future pending> attached to a different loop"
# https://github.com/encode/starlette/issues/1315#issuecomment-980784457
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To reproduce, run

GIT_TAG=$(git rev-parse HEAD) WORKSPACE=..  pytest -v -s tests/unit/api/test_tasks.py::test_create_streaming_task_success tests/unit/api/test_llms.py::test_completion_stream_success

When running these tests individually, we don't have an issue. Only when running both does this error appear. I put id(asyncio.get_running_loop()) throughout the create_completion_stream_task method and everything lined up and didn't error, so there must be something up with the TestClient library itself. Time-boxing this for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried the small fixes in encode/starlette#1315 (comment), none of them seemed to work. I think we will either need to downgrade fastapi as folks suggested.

@song-william song-william enabled auto-merge (squash) January 3, 2024 23:47
@song-william song-william merged commit 756682f into main Jan 4, 2024
5 checks passed
@song-william song-william deleted the wsong/sens-log-mode branch January 4, 2024 00:06
@yunfeng-scale yunfeng-scale mentioned this pull request Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants