feat: phase 1 bootstrap hardening (sqlite-first, migration, health)#32
Conversation
![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
This pull request transitions the storage layer to support both SQLite and PostgreSQL, establishing SQLite as the default for early development phases. Key changes include the introduction of a centralized storage management module, automated migration handling, and the addition of a readiness health check endpoint. Feedback highlights several improvement opportunities: enforcing referential integrity in the initial SQL migrations via foreign keys, expanding the readiness check to include Redis, and optimizing asynchronous performance by replacing blocking filesystem operations. Additionally, suggestions were made to refine configuration parsing for SQLite URLs and adjust logging levels for unmatched routes to reduce noise.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1520788a5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dependencies, | ||
| }; | ||
|
|
||
| Json(response) |
There was a problem hiding this comment.
readyz always returns Json<HealthResponse>, which Axum serializes with HTTP 200 even when database_ok is false and status is set to not_ready. In deployments that use /readyz for readiness probing, this will keep the instance in service while its database dependency is unavailable, so traffic can be routed to a node that has already declared itself unready in the payload.
Useful? React with 👍 / 👎.
| install_default_drivers(); | ||
|
|
||
| let pool = AnyPoolOptions::new() | ||
| .max_connections(10) |
There was a problem hiding this comment.
This path accepts sqlite://:memory: (see the explicit :memory: handling) but still creates an AnyPool with max_connections(10). For SQLite in-memory databases, each connection gets its own isolated database, so migrations can run on one connection while later queries hit another and fail with missing schema/data. This causes nondeterministic failures whenever tests or local runs use an in-memory URL.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f14e835c55
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| run_migrations: env::var("MIZAN_RUN_MIGRATIONS") | ||
| .unwrap_or_else(|_| "true".to_owned()) | ||
| .parse() | ||
| .unwrap_or(true), |
There was a problem hiding this comment.
MIZAN_RUN_MIGRATIONS is parsed with .parse().unwrap_or(true), so any non-lowercase-true/false value silently becomes true. In practice, operators commonly set values like FALSE or 0 to disable startup migrations; with this code, migrations still run and can execute DDL unexpectedly in environments where schema changes are managed separately.
Useful? React with 👍 / 👎.
| if !database_url.starts_with("sqlite://") { | ||
| return Ok(()); |
There was a problem hiding this comment.
sqlite: URLs in sqlite directory setup
The sqlite parent-directory bootstrap only runs for URLs starting with sqlite://, but SQLx also accepts sqlite: file URLs (for example sqlite:data/mizan.sqlite3?mode=rwc). With that valid format, this function returns early, no parent directory is created, and startup can fail with unable to open database file when the directory does not already exist.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 991de6945e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
✅ PR review-feedback fixes completed and pushed in commit
Validation run: I also resolved the two still-active review threads on this PR directly ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 895b3e6b69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Terupdate untuk review baru P2
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a53c5b2df
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32f928c96f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let hashed = bcrypt::hash(password, bcrypt::DEFAULT_COST) | ||
| .map_err(|error| AppError::infrastructure(format!("password hash failed: {error}")))?; |
There was a problem hiding this comment.
create_user runs bcrypt::hash directly on the async executor thread, and this path is used by the public /auth/register endpoint. Under concurrent sign-up traffic (or abuse), these CPU-heavy hashes can occupy Tokio worker threads and increase latency for unrelated requests. This should be moved to spawn_blocking (as done for password verification) so expensive hashing does not block the runtime.
Useful? React with 👍 / 👎.
1 participant
Summary
Validation
GET /healthzandGET /readyzon127.0.0.1:18180Notes
.gitignoremigrations/0001_initial.sql