Feat/add kodo config#56
Conversation
- StorageSettings (env prefix QINIU_) in windup_framework/config - add qiniu>=7.14 to framework deps (resolved 7.18.0) - tests for config loading/defaults/env prefix (no network) - client wrapper deferred to server/asset domain
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Found four correctness/documentation issues in the new backend bootstrap and database configuration. No performance-specific or Kodo-specific security findings remained after deduplication.
Validation: python3 -m compileall -q backend/packages backend/tests passed. The configured Ruff/import-linter/pytest suite could not be run because uv and the project dependencies are not installed in the review environment.
|
|
||
| async def get_session() -> Iterator[Session]: | ||
| """FastAPI 依赖:每请求一个 session,请求结束自动关闭。""" | ||
| async with SessionLocal() as session: |
There was a problem hiding this comment.
[P1] Use a consistent synchronous or asynchronous SQLAlchemy session API. SessionLocal creates a synchronous Session, which does not implement the async context-manager protocol, and its commit, rollback, and close methods are not awaitable. Consequently every request that resolves get_session (including /health/db) fails before the dependency can yield a session. Make this a normal generator using with SessionLocal() and synchronous calls, or switch the engine/session factory and consumers to SQLAlchemy's async APIs.
| def url(self) -> str: | ||
| """SQLAlchemy 连接串(psycopg3 驱动)。""" | ||
| return ( | ||
| f"postgresql+psycopg://{self.user}:{self.password}" |
There was a problem hiding this comment.
[P1] Build the database URL with escaping. Interpolating credentials directly makes valid passwords/usernames containing URL-reserved characters (for example @, :, /, or %) change how SQLAlchemy parses the connection string, so deployments with generated secrets can connect to the wrong host/database or fail outright. Construct a sqlalchemy.engine.URL (for example via URL.create) or percent-encode each component instead.
| factory=True, | ||
| host=os.getenv("WINDUP_HOST", "127.0.0.1"), | ||
| port=int(os.getenv("WINDUP_PORT", "8000")), | ||
| reload=bool(os.getenv("WINDUP_RELOAD")), |
There was a problem hiding this comment.
[P2] Parse WINDUP_RELOAD as a boolean value. bool() treats every non-empty string as true, so common settings such as WINDUP_RELOAD=false and WINDUP_RELOAD=0 unexpectedly enable reload mode. Compare against an explicit set of true values or use typed settings parsing.
| ``create_app`` 负责创建 FastAPI 实例并挂载路由 / 中间件 / 异常处理, | ||
| 是整个 web 服务的唯一装配点(composition root)。 | ||
|
|
||
| ``main`` 是开发启动入口:``python -m windup_app`` 或 ``windup`` 命令。 |
There was a problem hiding this comment.
[P2] Remove or implement the documented package entry point. python -m windup_app requires windup_app/__main__.py, but this PR only adds an empty __init__.py; the if __name__ == "__main__" block in this submodule does not make the package command work. Add __main__.py delegating to main(), or document the working windup/python -m windup_app.bootstrap.app invocation.
1 participant
背景
配置 七牛云 对象存储