refactor: lazy-init shared db for DATABASE_URL (#224)#234
Conversation
- Open SQLite on first db/dialect access via Proxy (no env read at import) - Replace test dynamic imports of tenant-db with static imports - tenant-db already resolved paths lazily in getTenantDbPath Made-with: Cursor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthrough複数のテストファイルで動的インポート（ Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
CodeRabbit（import hoist）への回答静的 また そのため、以前の |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/services/db.server.ts`:
- Around line 69-72: The closeDb() function is closing the same SQLite handle
twice because Kysely's SqliteDriver.destroy() already closes
sharedDbState.database; remove the extra sharedDbState.database.close() call so
destroy() is the sole resource cleanup. Apply the same fix to closeTenantDb()
and closeAllTenantDbs(): stop calling the underlying database.close() after
awaiting the Kysely destroy() and let destroy() handle closing the
better-sqlite3 handle.
In `@app/services/tenant-db.server.test.ts`:
- Around line 7-12: The import lines in this test use a relative path; replace
them with the project alias prefix by changing the module import to the
app-alias (e.g. import { closeAllTenantDbs, closeTenantDb, deleteTenantDb,
getTenantDb } from '~/services/tenant-db.server') so all references to
closeAllTenantDbs, closeTenantDb, deleteTenantDb and getTenantDb come from the
aliased path and conform to the guideline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b25ebe59-9a01-48d7-917c-765da21f5ed1
📒 Files selected for processing (10)
app/libs/timezone.server.test.tsapp/routes/$orgSlug/settings/github-users._index/mutations.server.test.tsapp/routes/$orgSlug/settings/repositories._index/mutations.server.test.tsapp/routes/$orgSlug/settings/repositories/$repository/settings/+functions/mutations.server.test.tsapp/routes/$orgSlug/settings/teams._index/mutations.server.test.tsapp/routes/$orgSlug/workload/$login/+functions/queries.server.test.tsapp/services/db.server.tsapp/services/tenant-db.server.test.tsbatch/db/mutations.test.tsbatch/github/store.test.ts
- SqliteDriver.destroy() already calls better-sqlite3 close() - Use ~/app import in tenant-db.server.test per project convention Made-with: Cursor
- Add test-only toOrgId helper next to OrganizationId brand - Deduplicate local toOrgId shims in integration tests Made-with: Cursor
1 participant
Summary
db.server.ts: Open the shared SQLite connection on firstdb/dialectaccess (Proxy), soDATABASE_URLis not read at module import time.await import('~/app/services/tenant-db.server')aftervi.stubEnvwith normal static imports (Issue refactor: tenant-db.server の DATABASE_URL 読み込みを遅延初期化に変更 #224 list +timezone.server.test,store.test,tenant-db.server.test).Why
db.server.tswas evaluatingDATABASE_URLat load time. Any test file that statically imported a module pulling indb.serverhad to use dynamic import ordering. Lazy init removes that constraint.Notes
tenant-db.server.tsalready readDATABASE_URLonly insidegetTenantDbPath(); no code change there.Closes #224
Made with Cursor
Summary by CodeRabbit
リリースノート
パフォーマンス改善
内部改善