feat(schema): add WorkerDeployment.externalId and task_runs_v2.external_deployment_id - #4661
feat(schema): add WorkerDeployment.externalId and task_runs_v2.external_deployment_id#46610ski wants to merge 1 commit into
Conversation
|
WalkthroughThe changes add Merge Risk: 🟡 Moderate · up to The new PostgreSQL index migration can fail when deployed through the default transactional path because concurrent index creation requires a non-transactional execution path. Merge readiness depends on adjusting the migration or deployment procedure. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
7625d61 to
603acad
Compare
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9f86bf16-4933-4e54-ad10-7f694b0ae9be
📒 Files selected for processing (4)
internal-packages/clickhouse/schema/040_add_task_runs_v2_external_deployment_id.sqlinternal-packages/database/prisma/migrations/20260818120000_add_worker_deployment_external_id/migration.sqlinternal-packages/database/prisma/migrations/20260818120100_add_worker_deployment_environment_id_external_id_index/migration.sqlinternal-packages/database/prisma/schema.prisma
🚧 Files skipped from review as they are similar to previous changes (1)
- internal-packages/database/prisma/schema.prisma
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (21)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 12)
- GitHub Check: runops-guard / runops-guard
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: fk-cascade-guard / fk-cascade-guard
- GitHub Check: code-quality / code-quality
- GitHub Check: audit
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build and publish previews
🧰 Additional context used
📓 Path-based instructions (2)
internal-packages/database/prisma/migrations/**/migration.sql
📄 CodeRabbit inference engine (internal-packages/database/CLAUDE.md)
internal-packages/database/prisma/migrations/**/migration.sql: When adding indexes to existing tables, useCREATE INDEX CONCURRENTLY IF NOT EXISTSto avoid production table locks.
Keep eachCONCURRENTLYindex in its own separate migration file, and add only one index per migration file.
Indexes on newly created tables may be created withoutCONCURRENTLYin the same migration asCREATE TABLE.
When adding an index for a new column on an existing table, use two migrations: firstALTER TABLE ... ADD COLUMN IF NOT EXISTS ..., then a separate migration containingCREATE INDEX CONCURRENTLY IF NOT EXISTS ....
Files:
internal-packages/database/prisma/migrations/20260818120000_add_worker_deployment_external_id/migration.sqlinternal-packages/database/prisma/migrations/20260818120100_add_worker_deployment_environment_id_external_id_index/migration.sql
internal-packages/clickhouse/schema/[0-9][0-9][0-9]_*.sql
📄 CodeRabbit inference engine (internal-packages/clickhouse/CLAUDE.md)
internal-packages/clickhouse/schema/[0-9][0-9][0-9]_*.sql: Migration file numbering: name files as0(N+1)_descriptive_name.sqlwhere N is the largest existing migration number inschema/; rebase and renumber if main adds migrations before opening a PR
DDL in migrations must be idempotent: useALTER TABLE ... ADD COLUMN IF NOT EXISTS,CREATE TABLE IF NOT EXISTS,DROP TABLE IF EXISTS,ADD INDEX IF NOT EXISTS,DROP INDEX IF EXISTS, andCREATE MATERIALIZED VIEW IF NOT EXISTSforms to allow out-of-order and retry-safe application
Files:
internal-packages/clickhouse/schema/040_add_task_runs_v2_external_deployment_id.sql
🧠 Learnings (2)
📚 Learning: 2026-02-03T18:48:31.790Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 2994
File: internal-packages/database/prisma/migrations/20260129162810_add_integration_deployment/migration.sql:14-18
Timestamp: 2026-02-03T18:48:31.790Z
Learning: For Prisma migrations targeting PostgreSQL: - When adding indexes to existing tables, create the index in a separate migration file and include CONCURRENTLY to avoid locking the table. - For indexes on newly created tables (in CREATE TABLE statements), you can create the index in the same migration file without CONCURRENTLY. This reduces rollout complexity for new objects while protecting uptime for existing structures.
Applied to files:
internal-packages/database/prisma/migrations/20260818120000_add_worker_deployment_external_id/migration.sqlinternal-packages/database/prisma/migrations/20260818120100_add_worker_deployment_environment_id_external_id_index/migration.sql
📚 Learning: 2026-03-22T13:49:20.068Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: internal-packages/database/prisma/migrations/20260318114244_add_prompt_friendly_id/migration.sql:5-5
Timestamp: 2026-03-22T13:49:20.068Z
Learning: For Prisma migration SQL files under `internal-packages/database/prisma/migrations/`, it is acceptable to create indexes with `CREATE INDEX` / `CREATE UNIQUE INDEX` (i.e., without `CONCURRENTLY`) when the parent table is introduced in the same PR and has no existing production rows yet. Only require `CREATE INDEX CONCURRENTLY` (or otherwise account for existing production data/locks) when the table already exists in production with data.
Applied to files:
internal-packages/database/prisma/migrations/20260818120000_add_worker_deployment_external_id/migration.sqlinternal-packages/database/prisma/migrations/20260818120100_add_worker_deployment_environment_id_external_id_index/migration.sql
🔇 Additional comments (2)
internal-packages/clickhouse/schema/040_add_task_runs_v2_external_deployment_id.sql (1)
1-7: LGTM!internal-packages/database/prisma/migrations/20260818120000_add_worker_deployment_external_id/migration.sql (1)
1-1: LGTM!
…al_deployment_id Migrations only, no code reads them yet. Postgres: nullable non-unique externalId on WorkerDeployment plus a CONCURRENTLY-built (environmentId, externalId) index in its own migration file. ClickHouse: external_deployment_id String DEFAULT '' on task_runs_v2 (plain String, not LowCardinality - commit SHAs are high-cardinality). Part of task run version skew protection (TRI-12998).
603acad to
aad440b
Compare
Migrations only, no code reads them yet. Postgres: nullable non-unique externalId on WorkerDeployment plus a CONCURRENTLY-built (environmentId, externalId) index in its own migration file. ClickHouse: external_deployment_id String DEFAULT '' on task_runs_v2 (plain String, not LowCardinality - commit SHAs are high-cardinality). Part of task run version skew protection (TRI-12998).