-
Notifications
You must be signed in to change notification settings - Fork 911
Use mysql database driver #1186
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
Conversation
WalkthroughSwitched drizzle backend from PlanetScale to mysql2 and renamed environment secret/variable to DATABASE_URL_MYSQL. Updated DB call result handling to destructure first element and use Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant API as Video Progress API
participant DB as DB (mysql2 + drizzle)
Note over API: Update progress
Client->>API: PATCH /api/desktop/.../video (progress)
API->>DB: UPDATE videoUploads SET progress=...
DB-->>API: [result] { affectedRows: N }
alt N > 0
API-->>Client: 204 No Content
else N == 0
Note over API,DB: No matching row — insert new upload record
API->>DB: INSERT INTO videoUploads (...)
DB-->>API: insert result
API-->>Client: 201 Created
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx,js,jsx,rs}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
⏰ 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). (3)
🔇 Additional comments (1)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
infra/sst.config.ts (1)
177-177
: Remove outdatedDATABASE_URL_HTTP
reference and sync environment types
- In
infra/sst-env.d.ts
(line 27), remove or rename theDATABASE_URL_HTTP
entry so it matches the newly addedDATABASE_URL_MYSQL
ininfra/sst.config.ts
.- Confirm
DATABASE_URL_MYSQL
is provisioned in all SST environments (staging, production) before deploying this change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
apps/web/actions/analytics/track-user-signed-up.ts
(2 hunks)apps/web/actions/organization/remove-invite.ts
(2 hunks)apps/web/actions/organization/remove-member.ts
(2 hunks)apps/web/app/api/desktop/[...route]/video.ts
(2 hunks)apps/web/app/api/upload/[...route]/multipart.ts
(2 hunks)apps/web/middleware.ts
(1 hunks)infra/sst.config.ts
(1 hunks)packages/database/auth/drizzle-adapter.ts
(1 hunks)packages/database/index.ts
(1 hunks)packages/database/package.json
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}
: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/app/api/upload/[...route]/multipart.ts
apps/web/app/api/desktop/[...route]/video.ts
apps/web/actions/organization/remove-invite.ts
apps/web/actions/analytics/track-user-signed-up.ts
apps/web/middleware.ts
apps/web/actions/organization/remove-member.ts
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/api/upload/[...route]/multipart.ts
apps/web/app/api/desktop/[...route]/video.ts
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
apps/web/app/api/upload/[...route]/multipart.ts
apps/web/app/api/desktop/[...route]/video.ts
apps/web/actions/organization/remove-invite.ts
apps/web/actions/analytics/track-user-signed-up.ts
apps/web/middleware.ts
apps/web/actions/organization/remove-member.ts
infra/sst.config.ts
packages/database/auth/drizzle-adapter.ts
packages/database/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}
: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format
.
Files:
apps/web/app/api/upload/[...route]/multipart.ts
apps/web/app/api/desktop/[...route]/video.ts
apps/web/actions/organization/remove-invite.ts
apps/web/actions/analytics/track-user-signed-up.ts
apps/web/middleware.ts
apps/web/actions/organization/remove-member.ts
infra/sst.config.ts
packages/database/auth/drizzle-adapter.ts
packages/database/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}
: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx
).
Use PascalCase for React/Solid components.
Files:
apps/web/app/api/upload/[...route]/multipart.ts
apps/web/app/api/desktop/[...route]/video.ts
apps/web/actions/organization/remove-invite.ts
apps/web/actions/analytics/track-user-signed-up.ts
apps/web/middleware.ts
apps/web/actions/organization/remove-member.ts
infra/sst.config.ts
packages/database/auth/drizzle-adapter.ts
packages/database/index.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQuery
oruseEffectMutation
from@/lib/EffectRuntime
; never callEffectRuntime.run*
directly in components.
Files:
apps/web/app/api/upload/[...route]/multipart.ts
apps/web/app/api/desktop/[...route]/video.ts
apps/web/actions/organization/remove-invite.ts
apps/web/actions/analytics/track-user-signed-up.ts
apps/web/middleware.ts
apps/web/actions/organization/remove-member.ts
apps/web/actions/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
All Groq/OpenAI calls must be implemented in Next.js Server Actions under apps/web/actions; do not place AI calls elsewhere
Files:
apps/web/actions/organization/remove-invite.ts
apps/web/actions/analytics/track-user-signed-up.ts
apps/web/actions/organization/remove-member.ts
🧬 Code graph analysis (6)
apps/web/app/api/upload/[...route]/multipart.ts (1)
packages/database/index.ts (1)
db
(13-20)
apps/web/app/api/desktop/[...route]/video.ts (1)
packages/database/index.ts (1)
db
(13-20)
apps/web/actions/organization/remove-invite.ts (1)
packages/database/index.ts (1)
db
(13-20)
apps/web/actions/analytics/track-user-signed-up.ts (1)
packages/database/index.ts (1)
db
(13-20)
apps/web/actions/organization/remove-member.ts (1)
packages/database/index.ts (1)
db
(13-20)
packages/database/auth/drizzle-adapter.ts (1)
packages/database/index.ts (1)
db
(13-20)
🪛 GitHub Check: Typecheck
packages/database/index.ts
[failure] 8-8:
Argument of type '[string | undefined]' is not assignable to parameter of type '[string | AnyMySql2Connection] | [string | AnyMySql2Connection, MySql2DrizzleConfig<Record<string, never>>] | [...]'.
[failure] 8-8:
Argument of type '[string | undefined]' is not assignable to parameter of type '[string | AnyMySql2Connection] | [string | AnyMySql2Connection, MySql2DrizzleConfig<Record<string, never>>] | [...]'.
⏰ 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). (3)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (11)
packages/database/package.json (2)
32-32
: LGTM! mysql2 dependency added.The mysql2 dependency has been correctly added to support the migration from PlanetScale to a MySQL2-based backend.
23-25
: Remove @mattrax/mysql-planetscale—unused in code; keep @planetscale/database (still imported in packages/database/index.ts) until the mysql2 migration is complete.apps/web/middleware.ts (1)
120-121
: LGTM! Node.js runtime required for mysql2.The addition of
runtime: "nodejs"
is necessary because mysql2 requires Node.js-specific features and cannot run in Edge runtime. This aligns with the migration to mysql2 in the database layer.packages/database/index.ts (1)
5-5
: LGTM! Correct import for mysql2.The import has been correctly updated to use
drizzle-orm/mysql2
instead of the PlanetScale-specific driver.apps/web/app/api/desktop/[...route]/video.ts (1)
351-371
: LGTM! Correct result handling for mysql2.The changes correctly adapt to mysql2's result structure:
- Destructuring
[result]
extracts the first element from the result array- Using
result.affectedRows
(mysql2 property) instead ofresult.rowsAffected
- Adding an insert fallback when no rows are affected provides upsert-like behavior
This pattern is consistent across the codebase changes in this PR.
apps/web/actions/organization/remove-invite.ts (1)
34-43
: LGTM! Consistent result handling for mysql2.The changes correctly update the delete result handling to match mysql2's structure:
- Destructuring
[result]
from the delete operation- Using
result.affectedRows
instead ofresult.rowsAffected
- Simplified error handling with a single-line guard
This aligns with the pattern used throughout this PR.
apps/web/actions/analytics/track-user-signed-up.ts (1)
36-45
: LGTM! Correct result handling for mysql2.The update result handling has been correctly adapted:
- Destructuring
[result]
from the update operation- Using
result.affectedRows
to determine if the update succeededThis ensures the tracking event only fires when the database update actually modifies a row, maintaining correct idempotency.
packages/database/auth/drizzle-adapter.ts (2)
4-4
: LGTM! Correct type import for mysql2.The import has been correctly updated to use
MySql2Database
type fromdrizzle-orm/mysql2
instead of PlanetScale-specific types.
10-10
: LGTM! Adapter signature updated for mysql2.The function signature has been correctly updated to accept
MySql2Database
instead ofPlanetScaleDatabase
. The adapter logic remains unchanged because drizzle-orm provides compatible interfaces across MySQL drivers.apps/web/app/api/upload/[...route]/multipart.ts (1)
303-322
: LGTM! Consistent result handling for mysql2.The update result handling has been correctly adapted to mysql2:
- Destructuring
[result]
from the update operation- Using
result.affectedRows
to verify the update succeeded- Conditionally deleting
videoUploads
only when the update affected rowsThe authentication check (line 318 comment) remains valid: if
affectedRows > 0
, the user owns the video and the update succeeded.apps/web/actions/organization/remove-member.ts (1)
54-63
: Delete result destructuring is correct for MySQL. Drizzle’s mysql2 adapter always returns an array whose first element is a metadata object containingaffectedRows
, soconst [result] = await db().delete(...);
is safe.
{ key: "CAP_AWS_BUCKET", value: recordingsBucket.bucket }, | ||
{ key: "NEXT_PUBLIC_CAP_AWS_BUCKET", value: recordingsBucket.bucket }, | ||
{ key: "DATABASE_URL", value: secrets.DATABASE_URL_HTTP.value }, | ||
{ key: "DATABASE_URL_MYSQL", value: secrets.DATABASE_URL_MYSQL.value }, |
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.
Verify environment variable name expected by the application.
The Vercel environment variable is being set with key DATABASE_URL_MYSQL
, but application code typically expects DATABASE_URL
. On line 291, the workflow cluster is configured with DATABASE_URL: secrets.DATABASE_URL_MYSQL.value
, suggesting the application expects DATABASE_URL
as the key.
Apply this diff if the application expects DATABASE_URL
:
- { key: "DATABASE_URL_MYSQL", value: secrets.DATABASE_URL_MYSQL.value },
+ { key: "DATABASE_URL", value: secrets.DATABASE_URL_MYSQL.value },
Alternatively, if the application has been updated to use DATABASE_URL_MYSQL
, verify with:
#!/bin/bash
# Description: Check which environment variable name is used in the application code
echo "=== Searching for DATABASE_URL usage in application code ==="
rg -n "process\.env\.DATABASE_URL[^_]|DATABASE_URL[^_]" apps/web/ packages/database/ --type ts
echo "=== Searching for DATABASE_URL_MYSQL usage ==="
rg -n "process\.env\.DATABASE_URL_MYSQL|DATABASE_URL_MYSQL" apps/web/ packages/database/ --type ts
🤖 Prompt for AI Agents
In infra/sst.config.ts around line 52, the environment variable key is set to
DATABASE_URL_MYSQL but the application (and another workflow entry at line 291)
expects DATABASE_URL; change the key to DATABASE_URL while keeping the secret
value (e.g., { key: "DATABASE_URL", value: secrets.DATABASE_URL_MYSQL.value })
and ensure any other env entries or workflow configs use the same DATABASE_URL
name so they match the application; if instead the app truly uses
DATABASE_URL_MYSQL, update the other workflow/config at line 291 to use
DATABASE_URL_MYSQL consistently.
function createDrizzle() { | ||
const URL = process.env.DATABASE_URL!; | ||
|
||
let fetchHandler: Promise<Config["fetch"]> | undefined; | ||
|
||
if (URL.startsWith("mysql://")) { | ||
fetchHandler = import("@mattrax/mysql-planetscale").then((m) => | ||
m.createFetchHandler(URL), | ||
); | ||
} | ||
|
||
const connection = new Client({ | ||
url: URL, | ||
fetch: async (input, init) => { | ||
return await ((await fetchHandler) || fetch)(input, init); | ||
}, | ||
}); | ||
|
||
return drizzle(connection); | ||
return drizzle(process.env.DATABASE_URL_MYSQL); | ||
} |
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.
Fix: DATABASE_URL_MYSQL must be validated as non-undefined.
Line 8 passes process.env.DATABASE_URL_MYSQL
directly to drizzle()
, but this value could be undefined
. TypeScript correctly flags this as a type error. If the environment variable is missing, the application will fail at runtime with cryptic errors.
Apply this diff to ensure the environment variable exists:
function createDrizzle() {
- return drizzle(process.env.DATABASE_URL_MYSQL);
+ const url = process.env.DATABASE_URL_MYSQL;
+ if (!url) {
+ throw new Error("DATABASE_URL_MYSQL environment variable is not set");
+ }
+ return drizzle(url);
}
Alternatively, if you're using a validated environment helper (like @cap/env
), use that instead:
import { serverEnv } from "@cap/env";
function createDrizzle() {
return drizzle(serverEnv().DATABASE_URL_MYSQL);
}
🧰 Tools
🪛 GitHub Check: Typecheck
[failure] 8-8:
Argument of type '[string | undefined]' is not assignable to parameter of type '[string | AnyMySql2Connection] | [string | AnyMySql2Connection, MySql2DrizzleConfig<Record<string, never>>] | [...]'.
[failure] 8-8:
Argument of type '[string | undefined]' is not assignable to parameter of type '[string | AnyMySql2Connection] | [string | AnyMySql2Connection, MySql2DrizzleConfig<Record<string, never>>] | [...]'.
🤖 Prompt for AI Agents
In packages/database/index.ts around lines 7 to 9, the call
drizzle(process.env.DATABASE_URL_MYSQL) may receive undefined; validate the
environment variable before passing it. Update the function to read the variable
into a local const, throw a clear error if it's undefined or empty, and then
pass that validated string to drizzle (or replace with your validated env helper
like serverEnv().DATABASE_URL_MYSQL if available).
Hopefully will help with out video delete problems
Summary by CodeRabbit
Bug Fixes
Refactor
Chores
Configuration