-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
fix: postgresql
protocol in database connection string assumes MongoDB database
#7688
Conversation
- This should allow developers to designate either postgres:// or postgresql:// prefixed URIs for PostgreSQL.
Thanks for opening this pull request!
|
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.
Can you please add a test for that protocol?
@@ -232,6 +232,12 @@ export function getDatabaseAdapter(databaseURI, collectionPrefix, databaseOption | |||
collectionPrefix, | |||
databaseOptions, | |||
}); | |||
case 'postgresql:': |
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.
Please use fall-through behavior instead of duplicating code
postgresql
protocol in database connection string assumes MongoDB database
Codecov Report
@@ Coverage Diff @@
## alpha #7688 +/- ##
==========================================
- Coverage 93.98% 93.87% -0.11%
==========================================
Files 183 183
Lines 13655 13625 -30
==========================================
- Hits 12833 12791 -42
- Misses 822 834 +12
Continue to review full report at Codecov.
|
Closed because already fixed as #7757 |
New Pull Request Checklist
Issue Description
This allows developers to use both valid PostgreSQL URI prefixes: "postgresql://" or "postgres://".
Reference: PostgreSQL Connection String Documentation
Related issue: #7687
Approach
I added a switch case for postgresql protocol to get the PostgresStorageAdapter.
TODOs before merging