-
-
Notifications
You must be signed in to change notification settings - Fork 288
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
chore: Update Sequelize to v6 #1085
chore: Update Sequelize to v6 #1085
Conversation
ec49380
to
cd75885
Compare
Codecov Report
@@ Coverage Diff @@
## master #1085 +/- ##
=======================================
Coverage 95.56% 95.56%
=======================================
Files 531 531
Lines 7123 7123
=======================================
Hits 6807 6807
Misses 316 316
Continue to review full report at Codecov.
|
Are you 100% sure the "underscored" option could not work like before? We use snake_case for all database columns, I'm not sure we want to move to camelCase. It's weird to mix snake_case and camelCase in same objects... |
cd75885
to
cb984f9
Compare
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.
Good enought for me.
Tested with existing database, and with blank one. All migrations are well applied.
Sorry. I mixed two issues. With underscore option, everything stays snake_case. |
Ah, I don't know if you saw my last PR, my I added this: https://github.com/GladysAssistant/Gladys/blob/master/server/index.js#L29 You say it's not needed anymore with Sequelize v6 ? From the doc, I see that it's still in the doc: |
I'm confuse. Documentation here https://sequelize.org/master/manual/connection-pool.html says that
and here https://sequelize.org/v5/manual/upgrade-to-v5.html#pooling that
But I let the close() I found in the shutdown file. https://sequelize.org/master/class/lib/sequelize.js~Sequelize.html#instance-method-close
|
They are talking about connection pooling only :) As we use Sqlite, there is no pooling (I guess), and I think closing the DB before stopping Gladys is safer to avoid letting the DB in a bad state |
Co-authored-by: Pierre-Gilles Leymarie <pierregilles.leymarie@gmail.com>
From v4 to v5:
From v5 to v6:
Due to sequelize/sequelize#11225, I added aliases for
created_at
andupdated_at
here : https://github.com/GladysAssistant/Gladys/pull/1085/files#diff-183dfcbb2c260fee7e3468389eaf038313e4ef51d52a3d8627590ba8daab4ce8R9Can you please double check
server/package-lock.json
? I don't know how to update only sequelize ?