[FEAT]: SSE를 위한 redis pub, sub 메시지 기능 추가 #154
Conversation
Summary of ChangesHello @Ochangmin524, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 SSE(Server-Sent Events)를 통해 사용자에게 실시간으로 작업 진행 상황을 알리기 위한 Redis 메시지 발행 기능을 추가합니다. 기존 Redis 클라이언트 구조를 재활용하여 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🤖 Gemini AI 코드 리뷰이 리뷰는 Gemini AI가 자동으로 생성했습니다. 참고용으로만 활용해주세요. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 SSE 알림을 위한 Redis pub/sub 기능을 추가하는 것을 목표로 합니다. 전반적인 방향은 좋지만, 코드의 안정성과 유지보수성을 높이기 위해 몇 가지 중요한 수정이 필요합니다.
주요 검토 의견은 다음과 같습니다:
- 전역 로깅 설정:
core/cache/redis_client.py에서logging.basicConfig를 호출하여 다른 모듈에 영향을 줄 수 있는 문제를 수정해야 합니다. NameError발생 가능성:report_consumer_impl_v2.py의 예외 처리 로직에서user_id변수가 할당되기 전에 참조될 수 있어NameError가 발생할 수 있는 심각한 버그가 있습니다.- 메시지 이중 인코딩: Redis에 발행되는 메시지가 JSON 형식으로 이중 인코딩되고 있어, 이를 수정하여 수신 측의 처리 과정을 단순화해야 합니다.
- 코드 중복: 여러 곳에서 반복되는 Redis 발행 로직을 별도의 헬퍼 메서드로 추출하여 코드의 가독성과 유지보수성을 개선하는 것이 좋습니다.
자세한 내용은 각 파일의 주석을 참고해 주세요.
| load_dotenv() | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
| logging.basicConfig(level=logging.INFO) |
There was a problem hiding this comment.
logging.basicConfig()를 라이브러리나 모듈의 최상위 레벨에서 호출하는 것은 좋지 않은 관행입니다. 이 코드는 루트 로거를 설정하여, 이 모듈을 임포트하는 모든 애플리케이션의 로깅 구성에 예기치 않은 영향을 줄 수 있습니다. 로깅 설정은 애플리케이션의 진입점(예: main.py)에서 한 번만 수행하는 것이 좋습니다. 이 줄을 삭제하는 것을 권장합니다.
| self._client = await RedisClient.get_instance() | ||
| return self._client | ||
|
|
||
| async def publish(self, user_id: str, message: str): |
There was a problem hiding this comment.
publish 메서드의 message 파라미터를 str 대신 dict 타입으로 받는 것이 좋습니다. 현재 report_consumer_impl_v2.py에서 json.dumps로 인코딩된 문자열을 전달하고 있는데, publish 메서드에서 다시 json.dumps를 호출하여 페이로드를 만들고 있습니다. 이로 인해 메시지가 이중으로 JSON 인코딩됩니다. ({"userId": "...", "message": "{\"status\": ...}"}) 이는 수신 측에서 파싱을 복잡하게 만듭니다. message 타입을 dict로 변경하고, 호출하는 쪽에서는 딕셔너리를 직접 전달하도록 수정하는 것을 권장합니다.
| async def publish(self, user_id: str, message: str): | |
| async def publish(self, user_id: str, message: dict): |
| await self.redis_service.publish( | ||
| user_id=str(user_id), | ||
| message=json.dumps({"status": "success", "step": "overview"}) | ||
| ) |
There was a problem hiding this comment.
RedisService.publish 메서드의 message 파라미터를 dict 타입으로 변경하는 것을 제안했습니다. 이에 따라 json.dumps를 호출할 필요 없이 딕셔너리 객체를 직접 전달해야 합니다. 이 수정은 이 파일의 다른 redis_service.publish 호출(155-158, 211-214, 225-228 라인)에도 동일하게 적용되어야 합니다.
| await self.redis_service.publish( | |
| user_id=str(user_id), | |
| message=json.dumps({"status": "success", "step": "overview"}) | |
| ) | |
| await self.redis_service.publish( | |
| user_id=str(user_id), | |
| message={"status": "success", "step": "overview"} | |
| ) |
| # redis 에 완료 메시지 보내기 | ||
| channel = await self.channel_repository.find_by_id(getattr(video, "channel_id", None)) | ||
| user_id = getattr(channel, 'member_id', None) | ||
|
|
||
| await self.redis_service.publish( | ||
| user_id=str(user_id), | ||
| message=json.dumps({"status": "success", "step": "overview"}) | ||
| ) |
There was a problem hiding this comment.
user_id를 조회하고 Redis에 상태를 발행하는 로직이 handle_overview_v2와 handle_analysis_v2 메서드, 그리고 각 메서드의 성공/실패 경로에 걸쳐 중복되고 있습니다. 코드 유지보수성을 높이기 위해 이 로직을 별도의 헬퍼(helper) 메서드로 추출하는 것을 고려해 보세요. 예를 들어, 다음과 같은 헬퍼 메서드를 만들 수 있습니다.
async def _publish_status(self, video: Optional[Any], user_id: Optional[str], status: str, step: str):
if user_id is None:
logger.warning(f"Cannot determine user_id to publish {step} status.")
return
await self.redis_service.publish(
user_id=str(user_id),
message={"status": status, "step": step}
)이렇게 하면 각 핸들러 메서드에서 간단하게 호출할 수 있어 코드가 더 깔끔해지고 중복을 줄일 수 있습니다.
PR 제목
[Feat/Fix/Refactor/Docs/Chore 등]: 간결하게 변경 내용을 요약해주세요. (예: Feat: 사용자 로그인 기능 구현)
✨ 변경 유형 (하나 이상 선택)
📚 변경 내용
구체적으로 어떤 변경 사항이 있는지, 왜 이러한 변경이 필요한지 설명해주세요.
(예: 사용자 회원가입 시 이메일 중복 확인 로직 추가. 기존 로직에서 누락된 부분 발견하여 수정.)
기존에 준이님께서, init으로 redis 만들어 주신 게 있어서, 싱글톤으로 만든 거 사용하기 위해서, redis 관련 파일을 하나로 합쳤습니다
사실 gpt 돌린거라서, 진짜 싱글톤인지는 모르겠습니다.
일단 로컬에서는 정상 작동 되어서 한 번 올려봅니다.
⚙️ 주요 작업 (선택 사항)
✅ 체크리스트
🔗 관련 이슈 (선택 사항)
해당 PR이 해결하는 이슈 또는 관련 있는 이슈가 있다면 링크를 걸어주세요.
(예: #123, ABC-456)
💬 기타 (선택 사항)
리뷰어에게 전달하고 싶은 추가 정보나 궁금한 점이 있다면 작성해주세요.