feat(deploy): 容器化后端 + /health 探针(已按评审拆出 CORS 到 #140) - #115
feat(deploy): 容器化后端 + /health 探针(已按评审拆出 CORS 到 #140)#115johnnyzhang-eng wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
状态同步一下,顺便说一个新出现的合并顺序问题。 本 PR 目前:5 个 commit,CI 全绿,FennoAI 两条评审都已修并逐条回复( 新增的顺序问题:#126 也改
本 PR 提供的正是这两样: 所以建议本 PR 先合——它只动部署面、范围小、CI 全绿;#126 rebase 后保留 env 入口,把 5174–5177 并进默认值即可。 已在 #126 的评审里同步说明了这一条。 |
|
/review -claude |
There was a problem hiding this comment.
Review — Deployable backend (PR #115)
Solid, well-reasoned PR. The CORS design (opt-in, scoped origin_regex instead of a hardcoded platform wildcard given allow_credentials=True), the same-path multi-stage Dockerfile, the MTU workaround, and the tests that pin dependency-declaration + CORS behavior are all correct and thoughtfully commented. Findings below are non-blocking; the docs mismatch and the two deployment-hardening items are the ones worth addressing.
Docs vs. reality
README.md:101-105— The README claims the frontend connects via a build-timeVITE_API_BASE_URLthat "启动直接报错" when unset. Verified there is noVITE_API_BASE_URL,import.meta.env, or any backend HTTP client reference anywhere underfrontend/(grep is empty). Neither the variable nor the fail-fast behavior exists yet, so operators following this section will build a frontend that is not wired to the backend. Either land the frontend wiring in this PR or mark the section as intended/not-yet-implemented. (See inline.)README.md:97—AI_BASE_URLis marked required with "默认 无", butframework/config/provider.pygivesbase_urla default (https://api.openai.com/v1); onlyAI_API_KEYis truly required. Minor table accuracy fix.docker-compose.yml:45wiresQINIU_PRIVATE_SPACE(defaultfalse) but it's absent from the README env table — consider adding it or noting the table is non-exhaustive.
Deployment / operational gap
- A fresh
docker compose upstarts cleanly and passes/health, but nothing in the repo provisions the DB schema (no Alembic, noBase.metadata.create_all, no startup lifespan — verified). The first DB-backed request will 500 on missing tables — exactly the "container healthy ≠ request succeeds" failure class this PR's tests guard against elsewhere. If schema setup is out-of-band, a note in compose/README would prevent surprise.
Additional concrete, line-level items are left as inline comments.
|
|
||
| ```bash | ||
| cd frontend | ||
| VITE_API_BASE_URL=http://<后端地址>:8000 npm run build |
There was a problem hiding this comment.
VITE_API_BASE_URL is documented (with a "未配置时启动直接报错" fail-fast guarantee) but not implemented: no VITE_API_BASE_URL, import.meta.env, or HTTP client reference exists anywhere under frontend/ (verified by grep). As written, following these steps produces a frontend build that is not actually wired to the backend. Either add the frontend wiring in this PR or flag this section as not-yet-implemented.
|
|
||
| WORKDIR /app | ||
|
|
||
| COPY --from=builder /app/.venv /app/.venv |
There was a problem hiding this comment.
The runtime stage never creates or switches to a non-root user, so uvicorn runs as root (UID 0). For a deployable image, consider dropping privileges to reduce blast radius if the app (e.g. the media-upload path) is compromised:
RUN useradd -r -u 1001 appuser && chown -R appuser /app
USER appuser| HEALTHCHECK --interval=30s --timeout=5s --retries=3 \ | ||
| CMD python -c "import urllib.request; urllib.request.urlopen('http://localhost:8000/health')" || exit 1 | ||
|
|
||
| CMD ["uvicorn", "windup_app.bootstrap.app:create_app", "--factory", "--host", "0.0.0.0", "--port", "8000"] |
There was a problem hiding this comment.
Single uvicorn process (default one worker). On a multi-core host this caps throughput and makes the service prone to head-of-line blocking on the sync Qiniu upload / Postgres paths. Consider env-driven concurrency (--workers/WEB_CONCURRENCY) for a deployable backend.
|
|
||
| # 打 /health 而不是 /docs:生产通常关掉交互文档(docs_url=None),那时探针会永远失败。 | ||
| HEALTHCHECK --interval=30s --timeout=5s --retries=3 \ | ||
| CMD python -c "import urllib.request; urllib.request.urlopen('http://localhost:8000/health')" || exit 1 |
There was a problem hiding this comment.
urlopen(...) has no timeout=. Docker's --timeout=5s kills the probe process, so it's mitigated, but relying on that is fragile — a hung socket otherwise blocks on Python's default (none). Prefer making intent explicit: urllib.request.urlopen('http://localhost:8000/health', timeout=4).
|
|
||
| FROM python:3.12-slim AS builder | ||
|
|
||
| COPY --from=ghcr.io/astral-sh/uv:latest /uv /usr/local/bin/uv |
There was a problem hiding this comment.
uv:latest is unpinned, which undercuts the reproducibility that uv.lock --frozen is trying to guarantee and can silently bust the dependency-install layer cache when upstream publishes. Pin to a specific uv version tag.
| POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:?请在 .env 里设置,不要用默认值} | ||
| POSTGRES_DB: ${POSTGRES_DB:-windup} | ||
| ports: | ||
| - "${POSTGRES_EXTERNAL_PORT:-7856}:5432" |
There was a problem hiding this comment.
Postgres is published to the host (default 0.0.0.0:7856). The backend reaches it over the internal windup-net bridge (POSTGRES_HOST: postgres), so this mapping isn't needed to run the app and exposes the DB on a cloud host without a strict firewall. Consider removing it, or binding to loopback: "127.0.0.1:${POSTGRES_EXTERNAL_PORT:-7856}:5432".
| POSTGRES_HOST: postgres | ||
| POSTGRES_PORT: 5432 | ||
| POSTGRES_USER: ${POSTGRES_USER:-root} | ||
| POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:?} |
There was a problem hiding this comment.
${POSTGRES_PASSWORD:?} here has an empty error message, unlike the helpful message on the postgres service (line 13). If service resolution order ever changes, the backend would fail with a blank, confusing error. Reuse the descriptive message for consistency.
如果目前只是为了解决本地开发时的跨域问题,单独加一个 CORS 中间件就足够了,建议把容器化相关的改动和 CORS 分开,或者更新一下 PR 描述,让意图更清晰。 |
…h the API 没有这个中间件,浏览器会把前端的每一个请求都拦在预检那一步: OPTIONS 返回 405、响应无 access-control-* 头,而后端日志里连请求都看不到 ——现场极易被误判成后端挂了。 - 来源由 WINDUP_CORS_ORIGINS 覆盖,默认值含 4173(vite preview,本地看真实 生产构建走这个端口)、5173(dev server)、3000 - 预览域名走 WINDUP_CORS_ORIGIN_REGEX 显式配置,**默认不开**:这里同时开了 allow_credentials,写死一条平台通配正则等于把带凭证的跨域请求放行给该平台上 任意第三方应用,且显式配了 WINDUP_CORS_ORIGINS 也关不掉它 四条断言进 CI。已用摘掉中间件的控制样本验证:其中三条在没有中间件时确实失败 (configured origin 拿到 405、4173 无放行头、regex 不生效),第四条是护栏断言。 Refs 1024XEngineer#139 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…l probe 让 main 上的后端能一条命令起在服务器上,并被容器健康检查正确判活。 本分支 stack 在 1024XEngineer#140(CORS)之上——两者都改 create_app(),1024XEngineer#140 合并后本分支 rebase。 - Dockerfile + docker-compose.yml:后端 + Postgres 一套起,uv sync --frozen 装依赖 - .dockerignore:构建上下文此前会带走 161MB 的本地虚拟环境 - GET /health 探针:HEALTHCHECK 原先打 /docs,而生产通常关掉交互文档 (docs_url=None),那时探针永远失败、容器被反复判死 - 声明 qiniu 依赖:media service 在函数体里延迟 import,不声明的话镜像能构建、 能启动、/docs 也正常,直到第一次 POST /media/upload 才 ModuleNotFoundError - README 补部署步骤与环境变量表 两条断言进 CI(find_spec("qiniu")、/health 可达)。 Refs 1024XEngineer#115 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
9e50476 to
bf932a2
Compare
|
@minorcell 谢谢,两条都收下了,按第一个选项拆了。 2. 拆分已完成。 你说得对,原标题「让 main 上的后端可直接部署并被浏览器访问」把两件事混在一句里,读起来像在做 CI/CD。现在:
补一句为什么 CORS 没有并进「本地开发」而是单独成 PR:它挡住的不只是本地——任何跑在浏览器里的前端都连不上后端,包括部署后的前端。而且它的症状很欺骗人:预检 1. CD 的方向我同意,本 PR 正是它的前置。 Actions 做 CD 也需要一个能构建的镜像和一份能起的 compose,本 PR 只把「能被部署」做进仓库,不含任何自动化部署,不阻塞 CD 的选型。 我会去找 @xiaocheny214 对一下 LAS 云主机的实际形态,有一个具体问题要先确认:compose 里我带了一个 Postgres 容器(自托管场景),如果 LAS 上用的是托管数据库,那段应该去掉、改成只跑后端容器 + 外部 |
- allow_origins=["*"] + allow_credentials=False;鉴权走 Authorization 头不走 cookie - 删 _cors_origin_regex(),收窄入口保留 WINDUP_CORS_ORIGINS - 删 backend/tests/test_cors.py Refs 1024XEngineer#139 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…l probe 让 main 上的后端能一条命令起在服务器上,并被容器健康检查正确判活。 本分支 stack 在 1024XEngineer#140(CORS)之上——两者都改 create_app(),1024XEngineer#140 合并后本分支 rebase。 - Dockerfile + docker-compose.yml:后端 + Postgres 一套起,uv sync --frozen 装依赖 - .dockerignore:构建上下文此前会带走 161MB 的本地虚拟环境 - GET /health 探针:HEALTHCHECK 原先打 /docs,而生产通常关掉交互文档 (docs_url=None),那时探针永远失败、容器被反复判死 - 声明 qiniu 依赖:media service 在函数体里延迟 import,不声明的话镜像能构建、 能启动、/docs 也正常,直到第一次 POST /media/upload 才 ModuleNotFoundError - README 补部署步骤与环境变量表 两条断言进 CI(find_spec("qiniu")、/health 可达)。 Refs 1024XEngineer#115 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
bf932a2 to
1987127
Compare
|
关闭。容器化与 compose 属于个人开发环境配置,不该进主仓,留在自己 fork 里自用。 正式部署按 LAS 云主机 + Actions CD 的方案走,与本 PR 的 compose 无关。 CORS 已拆到 #140,那条是产品需要(不挂中间件浏览器连不上后端),继续推进。 |
变更内容
让 main 上的后端能一条命令起在服务器上。
Dockerfile+docker-compose.yml:后端 + Postgres,uv sync --frozen装依赖.dockerignore:构建上下文此前带走 161MB 本地虚拟环境GET /health:HEALTHCHECK 原先打/docs,生产关掉交互文档后探针会永远失败qiniu:media service 函数体内延迟 import,不声明则首次POST /media/upload才ModuleNotFoundError不含 CI/CD。CD 待与 LAS 云主机形态对齐后单独提。
关联
Refs #115;stack 在 #140 之上。
本地验证
uv run ruff check .→ All checks passeduv run lint-imports→ Contracts: 2 kept, 0 brokenuv run pytest -q→ 3 passeduv lock --check→ Resolved 70 packages,lock 与 pyproject 同步待确认
compose 里带了 Postgres 容器(自托管场景)。若 LAS 用托管数据库,应改成只跑后端容器 + 外部
WINDUP_DB_URL。