-
Notifications
You must be signed in to change notification settings - Fork 21
[AN-257] Update mysql version to 5.7 #4804
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
… with checksum validations
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4804 +/- ##
===========================================
+ Coverage 74.77% 74.79% +0.02%
===========================================
Files 165 165
Lines 14951 14951
Branches 1187 1187
===========================================
+ Hits 11179 11182 +3
+ Misses 3772 3769 -3 see 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
lucymcnatt
left a 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.
Thanks for adding the comments!
aednichols
left a 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.
We should consider MySQL 8.4, which is the current LTS [0].
It buys us a lot more time than 8.0, which expires in July 2026; meaning it's only a 1.5 year extension over 5.6.
[0] The mysql:lts tag currently points to mysql:8.4
.github/workflows/unit_test.yml
Outdated
| services: | ||
| mysql: | ||
| image: mysql/mysql-server:5.6 | ||
| image: mysql/mysql-server:8.0 |
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.
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 call, let me try that
| db { | ||
| driver = "com.mysql.cj.jdbc.Driver" | ||
| url = "jdbc:mysql://"${mysql.host}":"${mysql.port}"/leotestdb?createDatabaseIfNotExist=true&useSSL=false&rewriteBatchedStatements=true&nullNamePatternMatchesAll=true" | ||
| url = "jdbc:mysql://"${mysql.host}":"${mysql.port}"/leotestdb?createDatabaseIfNotExist=true&allowPublicKeyRetrieval=true&useSSL=false&rewriteBatchedStatements=true&nullNamePatternMatchesAll=true" |
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.
I would love to enable SSL everywhere and disable allowPublicKeyRetrieval but that seems like a project for another PR.
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.
Yeah, I checked that this is only used for our unit testing at least
|
Haha lots of folks mad about this originally undocumented breaking change with mySQL 8.4: https://bugs.mysql.com/bug.php?id=114838 |
|
Grrr, driving me a bit nuts, the FK is referencing the primary key, how can it not be unique already? 🙄 |
Jira ticket: https://broadworkbench.atlassian.net/browse/AN-257
Summary of changes
What
Why
Testing these changes
What to test
Who tested and where
jenkins retestorjenkins multi-test.