-
Notifications
You must be signed in to change notification settings - Fork 0
Fix: 순수 ASGI 인터페이스로 수정 #123
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: develop
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughStarlette의 클래스 기반 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @wacruit/src/utils/middlewares.py:
- Around line 11-14: The middleware currently sets scope["scheme"] = "https" for
both HTTP and WebSocket in __call__, which is incorrect; change the logic so
that when scope["type"] == "http" set scope["scheme"] = "https" and when
scope["type"] == "websocket" set scope["scheme"] = "wss", preserving the call to
await self.app(scope, receive, send); reference the __call__ method,
scope["type"], scope["scheme"], and self.app to locate and update the code.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wacruit/src/utils/middlewares.py
🔇 Additional comments (2)
wacruit/src/utils/middlewares.py (2)
1-4: 순수 ASGI 인터페이스로의 임포트가 올바릅니다.BaseHTTPMiddleware 의존성을 제거하고 순수 ASGI 타입만 임포트하여 AnyIO 4.x 호환성 문제를 해결하는 접근 방식이 적절합니다.
7-9: 표준 ASGI 미들웨어 패턴을 올바르게 따릅니다.클래스 정의와 초기화가 ASGI 미들웨어 표준을 따르고 있으며, BaseHTTPMiddleware 제거를 통해 PR에서 언급한 호환성 문제를 해결합니다.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wacruit/src/utils/middlewares.py (1)
11-16: LGTM! 순수 ASGI 인터페이스 구현이 정확합니다.
__call__메서드가 ASGI 사양을 올바르게 따르고 있습니다:
- HTTP 요청에는 "https" scheme 설정
- WebSocket 연결에는 "wss" scheme 설정
- 다른 scope 타입(예: "lifespan")은 자동으로 변경 없이 통과
scope 직접 변경은 ASGI 미들웨어의 표준 패턴이므로 안전합니다.
♻️ 선택적 개선: 명시적인 lifespan 처리
가독성 향상을 위해 lifespan 이벤트를 명시적으로 처리하는 것을 고려해볼 수 있습니다 (현재 동작은 이미 올바릅니다):
async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: if scope["type"] == "http": scope["scheme"] = "https" elif scope["type"] == "websocket": scope["scheme"] = "wss" + # lifespan 및 기타 scope 타입은 변경 없이 통과 await self.app(scope, receive, send)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wacruit/src/utils/middlewares.py
🔇 Additional comments (2)
wacruit/src/utils/middlewares.py (2)
1-4: LGTM! 순수 ASGI 타입으로의 전환이 적절합니다.BaseHTTPMiddleware 의존성을 제거하고 순수 ASGI 타입(ASGIApp, Scope, Receive, Send)으로 변경한 것이 올바릅니다. 이를 통해 AnyIO 4.x와의 호환성 문제를 해결할 수 있습니다.
7-9: LGTM! 핵심 문제 해결을 위한 올바른 변경입니다.BaseHTTPMiddleware 상속을 제거하고 순수 ASGI 미들웨어 패턴으로 변경한 것이 적절합니다. 이것이 프로덕션 환경에서 발생했던 ExceptionGroup 처리 문제를 해결하는 핵심 변경사항입니다.
제미나이 답변