Skip to content
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

[WIP] Upgrade Knex to latest version and swap unsupported sqlite3 fork #12557

Closed
wants to merge 8 commits into from

Conversation

derrickmehaffy
Copy link
Member

@derrickmehaffy derrickmehaffy commented Feb 16, 2022

Signed-off-by: Derrick Mehaffy derrickmehaffy@gmail.com

Upgrades Knex to latest version (v1.0.3) and swaps out mapbox's node-sqlite3 for Microsoft's @vscode/sqlite3 instead

Also adds two new dialects from Knex:

  • better-sqlite3
  • mysql2

I've set the default in this PR to make better-sqlite3 the default instead of sqlite, we should consider dropping normal sqlite3 in the future.
mysql2 is required for MySQL v8+ and it's new auth mechanism, we should consider dropping mysql dialect in the future.

Signed-off-by: Derrick Mehaffy <derrickmehaffy@gmail.com>
@derrickmehaffy derrickmehaffy added issue: enhancement Issue suggesting an enhancement to an existing feature source: core:database Source is core/database package labels Feb 16, 2022
@codecov
Copy link

codecov bot commented Feb 16, 2022

Codecov Report

Merging #12557 (bdd9226) into master (6df5d8d) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master   #12557      +/-   ##
==========================================
- Coverage   47.78%   47.75%   -0.04%     
==========================================
  Files         231      231              
  Lines        8619     8625       +6     
  Branches     1922     1927       +5     
==========================================
  Hits         4119     4119              
- Misses       3701     3706       +5     
- Partials      799      800       +1     
Flag Coverage Δ
front ?
unit 47.75% <0.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/core/database/lib/dialects/index.js 15.78% <0.00%> (-1.86%) ⬇️
packages/core/database/lib/entity-manager.js 3.82% <0.00%> (ø)
packages/core/database/lib/query/helpers/search.js 14.28% <0.00%> (-0.72%) ⬇️
packages/core/database/lib/schema/schema.js 7.59% <0.00%> (-0.20%) ⬇️
...n/server/migrations/field/migrate-for-bookshelf.js 23.07% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6df5d8d...bdd9226. Read the comment docs.

@alexandrebodin alexandrebodin self-assigned this Feb 22, 2022
@alexandrebodin alexandrebodin added pr: enhancement This PR adds or updates some part of the codebase or features and removed issue: enhancement Issue suggesting an enhancement to an existing feature labels Mar 2, 2022
Signed-off-by: Derrick Mehaffy <derrickmehaffy@gmail.com>
Signed-off-by: Derrick Mehaffy <derrickmehaffy@gmail.com>
@derrickmehaffy derrickmehaffy changed the title Upgrade Knex to latest version and swap unsupported sqlite3 fork [WIP] Upgrade Knex to latest version and swap unsupported sqlite3 fork Mar 21, 2022
@derrickmehaffy
Copy link
Member Author

Needs more testing

Signed-off-by: Derrick Mehaffy <derrickmehaffy@gmail.com>
Signed-off-by: Derrick Mehaffy <derrickmehaffy@gmail.com>
Signed-off-by: Derrick Mehaffy <derrickmehaffy@gmail.com>
Signed-off-by: Derrick Mehaffy <derrickmehaffy@gmail.com>
@derrickmehaffy
Copy link
Member Author

super-seeded by: #12918

(Of course alex is a boss)

@gu-stav gu-stav deleted the feature/knex-v1 branch March 24, 2022 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: enhancement This PR adds or updates some part of the codebase or features source: core:database Source is core/database package
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants