-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add support for MySQL #7538
Add support for MySQL #7538
Conversation
🦋 Changeset detectedLatest commit: 3a80a4c The changes in this PR will be included in the next version bump. This PR includes changesets to release 15 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 3a80a4c:
|
For this, we should expect whatever it returns based on the database we're using. (Potentially in the future, we might want to have a consistent thing rather than what Prisma gives us but that is a totally separate thing to supporting MySQL)
I'm guessing that's because those tests create a huge number of lists. The fix here is probably going be to have a bunch of keystone instances with 1 list(or however many is convenient for how the tests are written) vs just a single keystone instance with a bunch of lists |
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.
looking great!
a few small suggestions for the guide
|
||
# Choosing the right Database | ||
|
||
Keystone supports Postgres, MySQL and SQLite as database engines for your backend. There might be different reasons why you would choose one over the other. |
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.
Should we link to something for Postgres
, MySQL
, SQLite
?
Perhaps other parts of our documentation that show how to configure each? 🤷
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.
Added some links...
- [Database Features](https://www.prisma.io/docs/reference/database-reference/database-features) | ||
- [Supported Databases](https://www.prisma.io/docs/reference/database-reference/supported-databases) | ||
|
||
!> **Note**: Keystone currently supports Postgres, MySQL, SQLite |
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.
Not sure we need this due to the presence in the top of page callout.
If you'd like to keep it, suggest including Prisma
somewhere to situate it in the Prisma H2 in a more obvious way.
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.
Thanks @raveling I have just rewritten this section
Docs update Co-authored-by: Ronald Aveling <luma.chroma@gmail.com>
Adding support for MySQL
Allows the setting of
db.provider
tomysql
inkeystone.ts
config.URL for MySQL as per Prisma connection details
Things to keep in mind when using MySQL - doc page added to cover this as well:
autoincrement
MySQL requires the field to also be indexed - added check to ensure this is set when using MySQL.mysql
to match currentpostgresql
testspostgres
tests are case sensitive andsqlite
skips thestring
tests which is also what this current PR does formysql
- this is not a blocker for this PR, however, an issue is logged - Un-skip filter tests #7574 - to complete this work. Also adding documentationnativeType
totext
field so this can be easily overridden.