fix(server): provide sqlite client before migration setup#5
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe SQLite persistence layer's Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f44e39f16c
ℹ️ 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".
| return Layer.provideMerge( | ||
| setup, | ||
| makeRuntimeSqliteLayer({ | ||
| filename: dbPath, |
There was a problem hiding this comment.
Pass setup as the dependent layer in provideMerge
In apps/server/src/persistence/Layers/Sqlite.ts, this call now uses the two-arg Layer.provideMerge overload with arguments reversed. In Effect, that overload is provideMerge(that, self), so putting makeRuntimeSqliteLayer(...) first feeds setup into the runtime layer instead of feeding the runtime SqlClient into setup. That leaves setup without SqlClient at build time and can reproduce the original Service not found: effect/sql/SqlClient startup failure (the same inversion is repeated in SqlitePersistenceMemory).
Useful? React with 👍 / 👎.
Motivation
setuplayer (PRAGMAs + migrations) required the EffectSqlClientservice but could run before the runtime sqlite client layer was provided, causingService not found: effect/sql/SqlClientduringdev.Description
makeRuntimeSqliteLayeris provided before thesetuplayer that usesSqlClient.makeSqlitePersistenceLiveand the in-memorySqlitePersistenceMemorylayer builders.apps/server/src/persistence/Layers/Sqlite.tsand only affectLayer.provideMergeordering.Testing
bun fmt,bun lint, andbun typecheck, but the execution environment lacks Bun (bun: command not found), so those automated checks could not be executed here.SqlClientservice will be available tosetupat runtime after this change.Codex Task
Summary by cubic
Ensure the runtime SQLite client layer is provided before migrations/setup so
effect/sql/SqlClientis available, fixing the "Service not found: effect/sql/SqlClient" error in dev.Reordered
Layer.provideMergeinmakeSqlitePersistenceLiveandSqlitePersistenceMemoryto providemakeRuntimeSqliteLayerbeforesetup.Written for commit f44e39f. Summary will update on new commits.
Summary by CodeRabbit