-
-
Notifications
You must be signed in to change notification settings - Fork 12
fix(cli): require datasource url for running migration and db push commands #431
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
WalkthroughA new validation utility function Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
Claude finished @ymc9's task —— View job PR Review: fix(cli): require datasource url for running migration and db push commandsReview Tasks
SummaryThis PR adds validation to ensure that a Code Quality & Best Practices ✅Strengths:
Minor suggestions:
Potential Issues ✅No major issues found
Performance Considerations ✅Minimal impact:
Security Concerns ✅No security issues identified:
Test Coverage Assessment
|
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.
Pull Request Overview
This PR adds validation to require a datasource URL before running migration and database push commands. The change prevents these commands from executing when the schema's datasource lacks a url field, which would cause failures during Prisma operations.
Key Changes:
- Added
requireDataSourceUrlfunction to validate datasource URL presence - Applied validation to migration commands (
migrate dev,reset,deploy,status,resolve) - Applied validation to database push command (
db push)
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/cli/src/actions/action-utils.ts | Introduces new requireDataSourceUrl validation function that checks for datasource URL field |
| packages/cli/src/actions/migrate.ts | Adds datasource URL validation before executing any migration commands |
| packages/cli/src/actions/db.ts | Adds datasource URL validation before executing database push command |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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: 0
🧹 Nitpick comments (1)
packages/cli/src/actions/action-utils.ts (1)
139-145: Consider improving the error message specificity.The current implementation conflates two distinct error cases:
- No datasource declaration exists in the schema
- Datasource exists but lacks a
urlfieldWhen no datasource is present,
dataSourceisundefined, and the error message "The schema's datasource must have a url field" is misleading.Apply this diff to provide clearer error messages:
export async function requireDataSourceUrl(schemaFile: string) { const zmodel = await loadSchemaDocument(schemaFile); const dataSource = zmodel.declarations.find(isDataSource); + if (!dataSource) { + throw new CliError('The schema must define a "datasource" to use this command.'); + } - if (!dataSource?.fields.some((f) => f.name === 'url')) { + if (!dataSource.fields.some((f) => f.name === 'url')) { throw new CliError('The schema\'s "datasource" must have a "url" field to use this command.'); } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/cli/src/actions/action-utils.ts(1 hunks)packages/cli/src/actions/db.ts(2 hunks)packages/cli/src/actions/migrate.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
packages/cli/src/actions/action-utils.ts (2)
packages/language/src/generated/ast.ts (1)
isDataSource(417-419)packages/cli/src/cli-error.ts (1)
CliError(4-4)
packages/cli/src/actions/migrate.ts (1)
packages/cli/src/actions/action-utils.ts (1)
requireDataSourceUrl(139-145)
packages/cli/src/actions/db.ts (1)
packages/cli/src/actions/action-utils.ts (1)
requireDataSourceUrl(139-145)
⏰ 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: Agent
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: build-test (20.x, postgresql)
🔇 Additional comments (4)
packages/cli/src/actions/db.ts (2)
3-3: LGTM!The import is correctly added and aligns with the usage in the
runPushfunction.
24-28: LGTM! Early validation improves error reporting.The datasource URL validation is correctly placed before schema generation, providing early feedback to users. The logic flow is clear and well-commented.
packages/cli/src/actions/migrate.ts (2)
5-5: LGTM!The import is correctly added and consistent with the usage in the
runfunction below.
37-40: LGTM! Validation gate protects all migration commands.The datasource URL validation is correctly positioned before schema generation and applies uniformly to all migration subcommands (dev, reset, deploy, status, resolve). The early validation ensures users receive clear feedback before any migration operations proceed.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.