-
Notifications
You must be signed in to change notification settings - Fork 1
DR-6844 Various create-db updates #76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
claim-db-worker | 2df4a28 | Commit Preview URL Branch Preview URL |
Jan 16 2026, 06:20 PM |
WalkthroughModularizes the create-db package: adds CLI handlers (create, regions), a Zod flags schema, CLI output utilities, a core services layer for worker interaction, multiple tests, import-path fixes, and bumps package versions. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (1)
✏️ Tip: You can disable this entire section by setting Comment |
|
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr76
npx create-pg@pr76
npx create-postgres@pr76Worker URLs
|
|
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr76
npx create-pg@pr76
npx create-postgres@pr76Worker URLs
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@create-db/__tests__/cli.test.ts`:
- Around line 6-17: Tests in create-db use the built CLI (CLI_PATH constant and
runCli helper) but no build step runs before tests; add a pretest script to
package.json that runs the existing build script (e.g., "pretest": "npm run
build" where "build" is "tsdown") so that dist/cli.mjs is created prior to
executing vitest, ensuring CLI_PATH points to a built file during test runs.
In `@create-db/__tests__/output.test.ts`:
- Around line 112-128: The flaky test that toggles POSIX permissions uses
fs.chmodSync on readOnlyDir and should be skipped on Windows; add a platform
guard at the start of the test (the "returns error for read-only directory"
block) that checks if process.platform === 'win32' and returns or calls
test.skip so the body (including fs.chmodSync and the writeEnvFile call) is not
executed on Windows, leaving the existing permission restore
(fs.chmodSync(readOnlyDir, 0o755)) and assertions unchanged for POSIX platforms;
reference the test name, readOnlyDir, fs.chmodSync, and writeEnvFile to locate
where to add the guard.
In `@create-db/__tests__/services.test.ts`:
- Around line 6-25: The live API test currently calls fetchRegions and asserts a
hard-coded "us-east-1", which is brittle; change the test in services.test.ts to
run only when an explicit integration flag is set (e.g.,
process.env.RUN_INTEGRATION_TESTS === "true") and otherwise skip it, and remove
or make the pinned region configurable by reading an env var (e.g.,
EXPECTED_REGION) and only assert the presence of that region if the env var is
provided; keep the generic assertions (Array.isArray, length > 0, shape checks
on region.id and region.status) unchanged and reference the fetchRegions call
and the regions variable when implementing the gating and optional region check.
In `@create-db/src/cli/commands/create.ts`:
- Around line 108-116: When input.json is true the code prints the JSON and
returns even on failure; update the logic in create.ts so that after calling
printJson(result) you check result.success and call process.exit(1) when false
instead of returning early. In short: in the branch guarded by input.json, still
printJson(result) but if result.success === false invoke process.exit(1) (use
the existing process.exit call) so shells receive a non‑zero exit code; refer to
input.json, printJson, result, and process.exit in your change.
🧹 Nitpick comments (4)
create-db/src/core/services.ts (1)
32-38: Extract process termination to CLI layer; avoid it in exported helpers.While
ensureOnlineis currently only called from the CLI, it's exported at the module level and could be misused by consumers who import directly fromservices.ts. Callingprocess.exit()in a utility function violates the principle of separation of concerns. Throw the error instead and let the CLI layer decide whether to exit, maintaining consistency with the programmatic API.Proposed fix
export async function ensureOnline() { try { await checkOnline(CREATE_DB_WORKER_URL); - } catch { + } catch (err) { await flushAnalytics(); - process.exit(1); + throw err; } }Then update CLI usages to handle the error:
- await ensureOnline(); + try { + await ensureOnline(); + } catch { + await flushAnalytics(); + process.exit(1); + }create-db/src/cli/commands/regions.ts (1)
6-15: Add error handling around region fetch for cleaner CLI UX.If the network call fails, the promise rejection will bubble up with a stack trace. Consider catching and printing a friendly error plus a non‑zero exit code.
♻️ Proposed fix
-import { fetchRegions } from "../../core/services.js"; +import { fetchRegions } from "../../core/services.js"; +import { printError } from "../output.js"; export async function handleRegions(): Promise<void> { - const regions = await fetchRegions(); - - log.message(""); - log.info(pc.bold(pc.cyan("Available Prisma Postgres regions:"))); - log.message(""); - for (const r of regions) { - log.message(` ${pc.green(r.id)} - ${r.name || r.id}`); - } - log.message(""); + try { + const regions = await fetchRegions(); + + log.message(""); + log.info(pc.bold(pc.cyan("Available Prisma Postgres regions:"))); + log.message(""); + for (const r of regions) { + log.message(` ${pc.green(r.id)} - ${r.name || r.id}`); + } + log.message(""); + } catch (err) { + printError(err instanceof Error ? err.message : String(err)); + process.exit(1); + } }create-db/src/cli/output.ts (1)
100-126: Restrict permissions when creating env files that contain credentials.
DATABASE_URLis sensitive; it’s safer to set a restrictive file mode on creation. This only applies when the file is first created (existing permissions remain unchanged).🔐 Proposed fix
- fs.appendFileSync(envPath, prefix + lines.join("\n"), { encoding: "utf8" }); + fs.appendFileSync(envPath, prefix + lines.join("\n"), { + encoding: "utf8", + mode: 0o600, + });create-db/src/index.ts (1)
44-49: Avoid hard‑coded CLI version drift.Line 48 hard-codes the CLI version string, which can easily fall out of sync with the package version. Consider sourcing it from the environment or package metadata.
♻️ Proposed refactor
@@ -export function createDbCli() { +const CLI_VERSION = process.env.npm_package_version ?? "1.1.0"; + +export function createDbCli() { return createCli({ router, name: getCommandName(), - version: "1.1.0", + version: CLI_VERSION, description: "Instantly create a temporary Prisma Postgres database", }); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
create-db/__tests__/cli.test.tscreate-db/__tests__/flags.test.tscreate-db/__tests__/output.test.tscreate-db/__tests__/services.test.tscreate-db/src/cli/commands/create.tscreate-db/src/cli/commands/index.tscreate-db/src/cli/commands/regions.tscreate-db/src/cli/flags.tscreate-db/src/cli/output.tscreate-db/src/core/database.tscreate-db/src/core/regions.tscreate-db/src/core/services.tscreate-db/src/index.tscreate-db/src/utils/analytics.tscreate-db/src/utils/env-utils.tscreate-db/src/utils/geolocation.ts
🧰 Additional context used
🧬 Code graph analysis (8)
create-db/__tests__/flags.test.ts (1)
create-db/src/cli/flags.ts (2)
CreateFlags(7-33)CreateFlagsInput(36-36)
create-db/src/cli/flags.ts (1)
create-db/src/index.ts (3)
CreateFlags(24-24)RegionSchema(23-23)CreateFlagsInput(24-24)
create-db/src/cli/commands/create.ts (5)
create-db/src/core/database.ts (1)
getCommandName(5-10)create-db/src/utils/env-utils.ts (1)
readUserEnvFile(4-23)create-db/src/core/services.ts (6)
sendAnalyticsEvent(21-27)ensureOnline(32-39)fetchRegions(45-47)flushAnalytics(13-13)validateRegionId(54-56)createDatabase(66-80)create-db/src/utils/geolocation.ts (2)
detectUserLocation(67-119)getRegionClosestToLocation(125-173)create-db/src/cli/output.ts (9)
showCancelled(19-21)printJson(73-75)printError(81-83)writeEnvFile(100-128)printSuccess(89-91)showIntro(9-11)createSpinner(27-34)printDatabaseResult(40-67)showOutro(14-16)
create-db/__tests__/output.test.ts (1)
create-db/src/cli/output.ts (1)
writeEnvFile(100-128)
create-db/src/core/services.ts (3)
create-db/src/utils/analytics.ts (2)
sendAnalytics(3-31)flushAnalytics(33-39)create-db/src/core/regions.ts (3)
checkOnline(4-19)getRegions(21-33)validateRegion(35-49)create-db/src/core/database.ts (1)
createDatabaseCore(12-154)
create-db/src/cli/commands/regions.ts (3)
create-db/src/cli/commands/index.ts (1)
handleRegions(2-2)create-db/src/index.ts (1)
regions(85-87)create-db/src/core/services.ts (1)
fetchRegions(45-47)
create-db/__tests__/services.test.ts (2)
create-db/src/index.ts (1)
regions(85-87)create-db/src/core/services.ts (1)
fetchRegions(45-47)
create-db/src/index.ts (6)
create-db/src/cli/flags.ts (1)
CreateFlags(7-33)create-db/src/cli/commands/create.ts (1)
handleCreate(29-178)create-db/src/cli/commands/index.ts (2)
handleCreate(1-1)handleRegions(2-2)create-db/src/cli/commands/regions.ts (1)
handleRegions(6-16)create-db/src/types.ts (3)
ProgrammaticCreateOptions(131-134)CreateDatabaseResult(64-64)Region(33-37)create-db/src/core/services.ts (2)
createDatabase(66-80)fetchRegions(45-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: create-db-worker
- GitHub Check: Workers Builds: claim-db-worker
🔇 Additional comments (10)
create-db/src/core/regions.ts (1)
2-2: LGTM — type import path aligned with new layout.create-db/src/utils/geolocation.ts (1)
1-6: LGTM — type import path update looks correct.create-db/src/core/database.ts (1)
2-3: LGTM — updated imports align with the new module layout.create-db/src/cli/flags.ts (1)
1-36: Solid flag schema and defaults.create-db/src/core/services.ts (4)
21-27: Nice, clean delegation for analytics.
45-47: LGTM — service wrapper is straightforward.
54-56: LGTM — simple validation delegate.
66-79: LGTM — createDatabase delegates cleanly to core.create-db/src/cli/commands/index.ts (1)
1-2: Re-exports look good.Clean, minimal surface for the CLI handlers.
create-db/__tests__/flags.test.ts (1)
5-221: Solid schema coverage.Lines 7-221 thoroughly validate defaults, valid/invalid inputs, combined parsing, and type inference. Nice coverage.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr76
npx create-pg@pr76
npx create-postgres@pr76Worker URLs
|
What was done:
src/cli/flags.tssrc/cli/commands/src/cli/output.tssrc/intocli/,core/, andutils/foldersBefore
After
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.