feat(ccip-server): migrate to Prisma 7 and ncc bundling#7895
feat(ccip-server): migrate to Prisma 7 and ncc bundling#7895paulbalaji merged 3 commits intomainfrom
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
ce8f2a3 to
ae751ae
Compare
ae751ae to
a1ec4a0
Compare
🔍 CCIP Server Docker Image Built SuccessfullyImage Tags: |
|
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. 📝 WalkthroughWalkthroughThis PR removes the standalone CCIP server Dockerfile, upgrades Prisma to version 7 with PostgreSQL adapter support, introduces bundling via ncc, and consolidates the service into the shared node-service Docker infrastructure with configurable port support. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@typescript/ccip-server/Dockerfile`:
- Around line 99-100: The RUN step installing
`@google-cloud/pino-logging-gcp-config` should pin a specific version to ensure
reproducible builds; update the Dockerfile's RUN npm install line that
references `@google-cloud/pino-logging-gcp-config` to include an explicit version
(matching your project's pinned toolset policy) rather than installing the
latest.
🧹 Nitpick comments (4)
typescript/ccip-server/package.json (1)
70-70: Consider movingprismaCLI to devDependencies.The
prismapackage is primarily the CLI tool used for migrations and generation during development/CI. Having it independenciesmeans it'll be included in production deployments unnecessarily. The@prisma/clientbelongs in dependencies (which you have), but the CLI itself is usually a dev-time thing.Since
postinstallrunsprisma generateand you're bundling for production, this shouldn't affect your workflow - the generation happens before bundling anyway.🧅 Suggested change
"dependencies": { ... - "prisma": "^7.3.0", ... }, "devDependencies": { ... + "prisma": "^7.3.0", ... }typescript/ccip-server/scripts/ncc.post-bundle.mjs (1)
36-45: These patches are a bit brittle but necessary for the WASM/Worker compatibility.The regex and string replacements are tightly coupled to ncc's current output format. If ncc or Prisma's bundled output changes shape, these could silently fail to match. Might be worth adding a warning if no replacements were made, so you know when Prisma or ncc updates break things.
🔧 Optional: Add match verification
+ let wasmPatchCount = 0; // Patch WASM init to use file:// URLs content = content.replace( /await __wbg_init\(\{ module_or_path: (module\$\d+) \}\)/g, - 'await __wbg_init({ module_or_path: pathToFileURL($1).href })', + (match, p1) => { + wasmPatchCount++; + return `await __wbg_init({ module_or_path: pathToFileURL(${p1}).href })`; + }, ); + const workerPatchApplied = content.includes('const worker = new Worker(url,'); // Patch Worker creation to accept file:// URLs content = content.replace( 'const worker = new Worker(url,', 'const worker = new Worker(pathToFileURL(url),', ); + if (wasmPatchCount === 0) { + console.warn('⚠ No WASM init patterns found - ncc output may have changed'); + } + if (!workerPatchApplied) { + console.warn('⚠ Worker pattern not found - ncc output may have changed'); + }typescript/ccip-server/src/db.ts (2)
6-8: Consider validating DATABASE_URL before creating the pool.Right now, if
DATABASE_URLis missing from the environment, this'll fail somewhere down the road when you actually try to query - probably with a confusing error message. It's like wandering into a swamp without checking if there's actually water in it.A quick check upfront would make debugging easier when things go sideways:
♻️ Suggested improvement
+const connectionString = process.env.DATABASE_URL; +if (!connectionString) { + throw new Error('DATABASE_URL environment variable is required'); +} + -const pool = new pg.Pool({ connectionString: process.env.DATABASE_URL }); +const pool = new pg.Pool({ connectionString }); const adapter = new PrismaPg(pool); export const prisma = new PrismaClient({ adapter });
6-6: Pool configuration is using defaults - verify this meets production needs.The
pg.Poolis created with just the connection string, which means you're getting the library defaults (max 10 connections, no idle timeout, etc.). This might be perfectly fine for your use case, but worth confirming it matches what's needed in production. Not everyone's swamp has the same traffic patterns.
a1ec4a0 to
9b9993b
Compare
9b9993b to
102162d
Compare
0815fbe to
edefef1
Compare
102162d to
144a6e5
Compare
9a57586 to
edefef1
Compare
144a6e5 to
dea4518
Compare
dea4518 to
562969c
Compare
|
Can we deploy the testnet offchain lookup server of this branch and do some test transfers on |
9287aa5 to
08de4f7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@typescript/Dockerfile.node-service`:
- Around line 134-135: The Dockerfile currently uses shell-style parameter
expansion in an EXPOSE line (EXPOSE ${SERVICE_PORT:-9090}), which Dockerfile
EXPOSE doesn't support; replace that line by either removing the
shell-substitution and relying on the unconditional EXPOSE 9090 (delete EXPOSE
${SERVICE_PORT:-9090}), or switch to a build ARG pattern (declare ARG
SERVICE_PORT with a default and then use EXPOSE ${SERVICE_PORT}) so Docker
interprets it correctly; alternatively document that services like rebalancer
and warp-monitor must publish ports at runtime with -p (or add explicit EXPOSE
entries per service such as EXPOSE 3000 for ccip-server) and ensure only valid
literal port numbers appear in EXPOSE lines.
🧹 Nitpick comments (1)
typescript/ccip-server/src/db.ts (1)
6-8: Consider adding some pool configuration to keep things from gettin' too crowded.The pool is created with just
connectionString, which means it'll usepg's defaults (typically 10 max connections). For a production service, ya might want to explicitly set pool options to avoid surprisin' behavior under load.♻️ Optional: Add pool configuration
-const pool = new pg.Pool({ connectionString: process.env.DATABASE_URL }); +const pool = new pg.Pool({ + connectionString: process.env.DATABASE_URL, + max: 20, // Maximum pool size + idleTimeoutMillis: 30000, + connectionTimeoutMillis: 5000, +});Also worth notin' — if
DATABASE_URLis undefined,pg.Poolwill still try to connect using defaults (localhost:5432). If you'd rather fail fast in that case:if (!process.env.DATABASE_URL) { throw new Error('DATABASE_URL environment variable is required'); }
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7895 +/- ##
=======================================
Coverage 77.02% 77.02%
=======================================
Files 117 117
Lines 2651 2651
Branches 244 244
=======================================
Hits 2042 2042
Misses 593 593
Partials 16 16
🚀 New features to boost your workflow:
|
⚙️ Node Service Docker Images Built Successfully
Full image paths |
🐳 Monorepo Docker Image Built Successfully
Full image paths |
Summary
Dockerfile.node-serviceWhy Prisma 7?
PR #7565 noted: "Once we upgrade to Node 24, we can upgrade to Prisma 7 which supports ncc bundling."
Prisma 7 eliminates the Rust query engine binary in favor of a TypeScript/WASM implementation, making it compatible with ncc bundling.
References:
Key migration changes:
prisma-client-js→prisma-client@prisma/adapter-pgfor PostgreSQLschema.prismatoprisma/config.tsImage Size Comparison
ccip-server(pnpm deploy, Prisma 6)ccip-server(ncc bundle, Prisma 7)Dockerfile Unification
This PR also unifies ccip-server into the shared
Dockerfile.node-service:SERVICE_PORTarg for HTTP servicesncc-servicesmatrix indocker-bake.hcltypescript/ccip-server/DockerfileChanges
typescript/ccip-server/prisma/schema.prisma- Prisma 7 generatortypescript/ccip-server/prisma/config.ts- Prisma 7 datasource configtypescript/ccip-server/package.json- Prisma 7 deps + bundle scripttypescript/ccip-server/src/db.ts- Prisma 7 adapter patterntypescript/ccip-server/scripts/ncc.post-bundle.mjs- Bundle patchingtypescript/Dockerfile.node-service- Prisma support + SERVICE_PORTtypescript/docker-bake.hcl- Add ccip-server to matrixDeleted Files
typescript/ccip-server/DockerfileStacked on #7881
Test Plan
pnpm installsucceeds with Prisma 7prisma generatesucceedspnpm turbo run bundle --filter=@hyperlane-xyz/ccip-serversucceeds🤖 Generated with Claude Code
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.