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

Admin only for logs websocket #1826

Merged
merged 1 commit into from
Jan 15, 2025
Merged

Admin only for logs websocket #1826

merged 1 commit into from
Jan 15, 2025

Conversation

NolanTrem
Copy link
Collaborator

@NolanTrem NolanTrem commented Jan 15, 2025

Important

Added WebSocket authentication for superusers in auth.py and updated r2r service image in Docker Compose.

  • WebSocket Authentication:
    • Added websocket_auth_wrapper in auth.py to restrict WebSocket access to superusers.
    • Applied websocket_auth_wrapper to stream_logs in logs_router.py.
  • Configuration Changes:
    • Updated base_model in r2r_azure.toml from openai/text-embedding-3-small to azure/text-embedding-3-small.
  • Docker Compose:
    • Changed r2r service image in compose.full.yaml from r2r/test to ${R2R_IMAGE:-ragtoriches/prod:latest}.

This description was created by Ellipsis for 4d3ed76. It will automatically update as commits are pushed.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 4d3ed76 in 1 minute and 9 seconds

More details
  • Looked at 124 lines of code in 4 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. py/core/main/api/v3/logs_router.py:77
  • Draft comment:
    Handle the case where auth_user is None to ensure proper error handling if authentication fails.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_0oEasOxYFq8Bsx3o


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@@ -208,6 +208,63 @@ async def _auth_wrapper(

return _auth_wrapper

def websocket_auth_wrapper(
Copy link
Contributor

Choose a reason for hiding this comment

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

The websocket_auth_wrapper function does not handle API key authentication, unlike the auth_wrapper function. Consider adding API key support for consistency.

@NolanTrem NolanTrem merged commit 3c36ffe into main Jan 15, 2025
14 checks passed
@NolanTrem NolanTrem deleted the Nolan/AdminLogs branch January 15, 2025 00:18
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.

1 participant