-
Notifications
You must be signed in to change notification settings - Fork 45
refactor: decouple transport and session management #135
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the code to decouple transport and session management while adding new modules to support SSE transport and session handling.
- Refactored error log file closing in errlog_manager.py
- Added new transport, session, and middleware modules to support SSE communication
- Reworked app initialization to integrate the new transport and session management
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/mcpm/utils/errlog_manager.py | Uses a copied list of keys to safely close log files |
src/mcpm/core/router/transport.py | Implements SSE transport handling |
src/mcpm/core/router/session.py | Introduces LocalSessionStore and SessionManager |
src/mcpm/core/router/middleware.py | Creates a middleware to inject session metadata |
src/mcpm/core/router/app.py | Updates the application to use the new transport layer |
def exist(self, session_id: UUID) -> bool: | ||
return self.session_store.exist(session_id) | ||
|
||
async def create_session(self, meta: dict[str, Any] = {}) -> Session: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using a mutable default argument for 'meta'. Consider using 'meta: Optional[dict[str, Any]] = None' and initializing it within the function.
async def create_session(self, meta: dict[str, Any] = {}) -> Session: | |
async def create_session(self, meta: Optional[dict[str, Any]] = None) -> Session: | |
if meta is None: | |
meta = {} |
Copilot uses AI. Check for mistakes.
async def handle_sse(request: Request): | ||
try: | ||
async with transport.connect_sse(request.scope, request.receive, request._send) as (read, write): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using the private attribute 'request._send'. Consider passing the 'send' parameter from the ASGI call, or refactoring to obtain a public interface.
async def handle_sse(request: Request): | |
try: | |
async with transport.connect_sse(request.scope, request.receive, request._send) as (read, write): | |
async def handle_sse(request: Request, send: Send): | |
try: | |
async with transport.connect_sse(request.scope, request.receive, send) as (read, write): |
Copilot uses AI. Check for mistakes.
No description provided.