Skip to content

Fix/cloudrun deployment#35

Merged
SanghunYun95 merged 88 commits intomainfrom
fix/cloudrun-deployment
Mar 29, 2026
Merged

Fix/cloudrun deployment#35
SanghunYun95 merged 88 commits intomainfrom
fix/cloudrun-deployment

Conversation

@SanghunYun95
Copy link
Copy Markdown
Owner

@SanghunYun95 SanghunYun95 commented Mar 29, 2026

Summary by CodeRabbit

릴리스 노트

  • 기능 개선

    • 채팅 엔드포인트의 분당 요청 제한 제거로 응답성 향상
  • 버그 수정

    • CORS 설정을 명시적 출처 목록으로 업데이트
    • Firebase 호스팅 배포 대상 설정 수정
  • 운영

    • 백엔드 시작 시 필수 설정 검증 강화
    • 컨테이너 프록시 IP 처리 옵션 추가

… (deploy.yml, Dockerfile, ingest_data.py, update_metadata.py)
…le exec, fail-fast config, .dockerignore cleanup)
…sh language consistency, non-intrusive fail-fast validation)
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 29, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
philo-rag Ready Ready Preview, Comment Mar 29, 2026 1:42am

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 29, 2026

📝 Walkthrough

🐰 풀 리퀘스트 분석

Walkthrough

백엔드 CORS 설정을 와일드카드에서 명시적 원본 목록으로 변경하고, 시작 시간 검증 로직을 함수로 리팩토링했으며, Firebase 호스팅 사이트 식별자를 업데이트하고, 챗 엔드포인트에서 속도 제한을 제거하고, 프록시 헤더 전달 설정을 추가했습니다.

Changes

Cohort / File(s) Summary
Core Configuration & Startup
backend/app/core/config.py, backend/app/main.py
모듈 수준의 필수 설정 검증을 새로운 validate_required_settings() 함수로 추출하고, 애플리케이션 시작 시 해당 함수를 호출하도록 변경. CORS 설정에서 allow_credentials를 True에서 False로 변경.
Rate Limiting 제거
backend/app/api/routes/chat.py
chat_endpointchat_title_endpoint 라우트에서 @limiter.limit("5/minute") 데코레이터 제거, 속도 제한 미적용.
배포 및 인프라
backend/Dockerfile, frontend/firebase.json, analysis_results.md
uvicorn 시작 명령에 --forwarded-allow-ips='*' 옵션 추가. Firebase 호스팅 사이트 식별자를 "vigilant-shift-490601-t5"에서 "philo-rag"로 변경.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 설정을 정리하고, 제한을 풀어주니,
Firebase는 새 이름으로 반짝이고,
신뢰도 기반이 단단해졌어요.
프록시는 더 똑똑해지고, 속도 제한은 사라졌고,
우리의 작은 배포가 더 빠르게 춤을 춘답니다! 🚀✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 pull request의 주요 변경사항을 명확히 요약합니다. 여러 파일(firebase.json, main.py, Dockerfile, config.py, chat.py)의 변경이 모두 CloudRun 배포 환경에 맞춘 설정 수정과 관련되어 있으며, 제목 'Fix/cloudrun deployment'는 이러한 전체 변경의 핵심 목적을 정확히 반영합니다.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/cloudrun-deployment
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch fix/cloudrun-deployment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
backend/app/main.py (1)

15-15: 중복된 asyncio import를 제거하세요.

asyncio가 9번 줄과 15번 줄에서 두 번 import되고 있습니다.

🧹 제안된 수정
-import asyncio
-
 `@asynccontextmanager`
 async def lifespan(_app: FastAPI):
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/app/main.py` at line 15, 파일 상단에서 두 번 선언된 asyncio 임포트를 정리하세요: 중복된
"import asyncio" 중 하나를 제거하여 모듈이 한 번만 임포트되도록 하고, 관련 코드(예: 비동기 함수나 이벤트 루프 사용 위치)는
그대로 유지해 동작에 영향이 없도록 합니다. 참조 심볼: asyncio.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@analysis_results.md`:
- Line 35: 문서의 설명과 실제 코드가 불일치하므로 analysis_results.md의 해당 문장을 backend에서 실제로 사용 중인
CORS 설정(즉 FastAPI/CORSMiddleware 호출에 allow_origins=["*"]가 여전히 사용되고 있고
allow_credentials=False로만 변경된 상태)을 반영하도록 수정하세요; 구체적으로 "Replaced the wildcard
`["*"]` with an explicit list of origins" 문구를 제거하거나 "Kept wildcard `["*"]` and
changed `allow_credentials` to `False`" 같은 정확한 설명으로 바꾸고, 문서 내에서 참조되는
심볼(allow_origins, allow_credentials, CORSMiddleware/FastAPI CORS 설정)과 일치하도록
업데이트하세요.
- Line 28: The documented Dockerfile CMD line is missing the
--forwarded-allow-ips='*' flag present in the actual backend Dockerfile; update
the documentation's CMD example that shows "CMD [\"sh\", \"-c\", \"exec uvicorn
app.main:app --host 0.0.0.0 --port ${PORT:-8080} --proxy-headers\"]" to include
--forwarded-allow-ips='*' so it matches the backend/Dockerfile (i.e., ensure the
uvicorn invocation includes --forwarded-allow-ips='*' alongside
--proxy-headers).

---

Nitpick comments:
In `@backend/app/main.py`:
- Line 15: 파일 상단에서 두 번 선언된 asyncio 임포트를 정리하세요: 중복된 "import asyncio" 중 하나를 제거하여
모듈이 한 번만 임포트되도록 하고, 관련 코드(예: 비동기 함수나 이벤트 루프 사용 위치)는 그대로 유지해 동작에 영향이 없도록 합니다. 참조
심볼: asyncio.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bec58dad-568f-477f-8991-9c57cb21f549

📥 Commits

Reviewing files that changed from the base of the PR and between 3d1acee and 047803e.

📒 Files selected for processing (6)
  • analysis_results.md
  • backend/Dockerfile
  • backend/app/api/routes/chat.py
  • backend/app/core/config.py
  • backend/app/main.py
  • frontend/firebase.json
💤 Files with no reviewable changes (1)
  • backend/app/api/routes/chat.py

Comment thread analysis_results.md
# ...

# Command to run the application using the PORT environment variable
CMD ["sh", "-c", "exec uvicorn app.main:app --host 0.0.0.0 --port ${PORT:-8080} --proxy-headers"]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

문서와 실제 Dockerfile이 불일치합니다.

문서의 Dockerfile CMD 예시에 --forwarded-allow-ips='*' 플래그가 누락되었습니다. 실제 backend/Dockerfile에는 이 플래그가 포함되어 있으므로, 향후 유지보수 시 혼란을 방지하기 위해 문서를 업데이트해 주세요.

📝 제안된 수정
-CMD ["sh", "-c", "exec uvicorn app.main:app --host 0.0.0.0 --port ${PORT:-8080} --proxy-headers"]
+CMD ["sh", "-c", "exec uvicorn app.main:app --host 0.0.0.0 --port ${PORT:-8080} --proxy-headers --forwarded-allow-ips='*'"]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
CMD ["sh", "-c", "exec uvicorn app.main:app --host 0.0.0.0 --port ${PORT:-8080} --proxy-headers"]
CMD ["sh", "-c", "exec uvicorn app.main:app --host 0.0.0.0 --port ${PORT:-8080} --proxy-headers --forwarded-allow-ips='*'"]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@analysis_results.md` at line 28, The documented Dockerfile CMD line is
missing the --forwarded-allow-ips='*' flag present in the actual backend
Dockerfile; update the documentation's CMD example that shows "CMD [\"sh\",
\"-c\", \"exec uvicorn app.main:app --host 0.0.0.0 --port ${PORT:-8080}
--proxy-headers\"]" to include --forwarded-allow-ips='*' so it matches the
backend/Dockerfile (i.e., ensure the uvicorn invocation includes
--forwarded-allow-ips='*' alongside --proxy-headers).

Comment thread analysis_results.md
- **Updated `site` ID**: Changed from the default project ID to `philo-rag` to deploy correctly to the `philo-rag.web.app` URL.

### 3. Update `backend/app/main.py`
- **Explicit CORS Origins**: Replaced the wildcard `["*"]` with an explicit list of origins. Browsers reject `*` when `allow_credentials=True` is used, so defining the specific Firebase and localhost URLs was necessary to fix the "Failed to fetch" errors.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

문서 내용이 실제 코드와 일치하지 않습니다.

문서에는 "Replaced the wildcard ["*"] with an explicit list of origins"라고 되어 있지만, 실제 backend/app/main.py에서는 allow_origins=["*"]가 그대로 유지되고 allow_credentials=False로만 변경되었습니다. 문서를 실제 변경 사항에 맞게 수정해 주세요.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@analysis_results.md` at line 35, 문서의 설명과 실제 코드가 불일치하므로 analysis_results.md의
해당 문장을 backend에서 실제로 사용 중인 CORS 설정(즉 FastAPI/CORSMiddleware 호출에
allow_origins=["*"]가 여전히 사용되고 있고 allow_credentials=False로만 변경된 상태)을 반영하도록 수정하세요;
구체적으로 "Replaced the wildcard `["*"]` with an explicit list of origins" 문구를 제거하거나
"Kept wildcard `["*"]` and changed `allow_credentials` to `False`" 같은 정확한 설명으로
바꾸고, 문서 내에서 참조되는 심볼(allow_origins, allow_credentials, CORSMiddleware/FastAPI
CORS 설정)과 일치하도록 업데이트하세요.

@SanghunYun95 SanghunYun95 merged commit f58eab1 into main Mar 29, 2026
3 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Mar 29, 2026
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