-
-
Notifications
You must be signed in to change notification settings - Fork 127
fix: make datasource url optional #2305
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
📝 WalkthroughWalkthroughValidator behavior changed: when a DataSource is missing the Changes
Sequence Diagram(s)sequenceDiagram
participant SchemaAST as Schema AST
participant Validator as DataSource Validator
rect rgba(220,235,255,0.6)
note right of SchemaAST: Incoming DataSource node
end
SchemaAST->>Validator: validateDataSource(node)
alt url field present
Validator->>Validator: validate url and shadowDatabaseUrl
Validator-->>SchemaAST: emit errors/warnings (if any)
else url field missing (OLD)
Validator->>Validator: emit error ("url field missing")
Validator-->>SchemaAST: emit error
else url field missing (NEW)
Validator-->>SchemaAST: return early (no url error)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ 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 |
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/schema/src/language-server/validator/datasource-validator.ts(1 hunks)
⏰ 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). (4)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: build-test (20.x)
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/schema/tests/schema/validation/datasource-validation.test.ts (1)
83-99: Add test coverage for optional url scenario.The success tests all include the url field. Consider adding a test case that validates a datasource with only the provider field (no url) to ensure the optional url feature works correctly:
it('success without url', async () => { await loadModel(` datasource db { provider = "postgresql" } `); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/schema/tests/schema/validation/datasource-validation.test.ts(1 hunks)tests/integration/tests/cli/generate.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/integration/tests/cli/generate.test.ts (1)
packages/testtools/src/schema.ts (1)
installPackage(68-70)
⏰ 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). (4)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: build-test (20.x)
🔇 Additional comments (2)
packages/schema/tests/schema/validation/datasource-validation.test.ts (1)
13-13: The test change is correct and verified.The validator implementation confirms that the
urlfield is optional. In thevalidateUrl()method (lines 40-48), when theurlfield is not present, the validator returns early without raising an error. ThevalidateProvider()method remains the only validator that will produce an error when the provider field is missing.The test now correctly expects a single error message about the missing provider field, which aligns with the validator's behavior where only the provider is required.
tests/integration/tests/cli/generate.test.ts (1)
48-48: The review comment is based on an incorrect premise and should be disregarded.Prisma 6.19 does not support optional datasource URLs—the
urlfield is required in Prisma 6.x. The test schema itself explicitly provides a datasource URL (url = "file:./dev.db"), so it does not depend on optional URL support. The reason for pinning to version 6.19.x remains undocumented in the code, but it is not for optional URL field support as the original review suggested.Likely an incorrect or invalid review 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/schema/tests/generator/prisma-generator.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/schema/tests/generator/prisma-generator.test.ts (1)
packages/schema/src/utils/exec-utils.ts (1)
execSync(6-10)
⏰ 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). (4)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
|
FYI, for Prisma 7 the url has been deprecated completely and this change is required to upgrade to v7. |
|
Thanks @jamiter . The current plan is to not support v7 for ZenStack v2. I'll share this information with the next release and collect feedback. |
No description provided.