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

Supply mysql8.4.0 by default #1071

Merged
merged 6 commits into from
Jun 7, 2024

Conversation

rohan-saeed
Copy link
Contributor

@rohan-saeed rohan-saeed commented May 26, 2024

fixes #1051
MySQL 8.4.0 LTS has been released. It is set as default in the settings.

Following things has been tested (on dev and k8s) and working perfectly:
Ability to create user and admin user
Create a course and its visibility on LMS
Ability to login to MySQL with native root user.

@@ -45,6 +45,7 @@ services:
--character-set-server=utf8mb3
--collation-server=utf8mb3_general_ci
--binlog-expire-logs-seconds=259200
--mysql-native-password=ON
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Reference for this change?
  • This would be needed in k8s as well.

Copy link
Contributor Author

@rohan-saeed rohan-saeed May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plugin mysql-native-password is disable by default in mysql 8.4.0 that makes us unable to authenticate with user having native password, such as the root user in our case. See here for more context.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option seems to be deprecated and during my testing, I found that removing this did not effect anything in my tutor installation. Can you confirm once again if this is required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I have tested it again. We are able to authenticate with root user without the plugin.

Copy link
Contributor

@DawoudSheraz DawoudSheraz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add some testing instructions. List some of the things that have been tested.

Copy link
Contributor

@Danyal-Faheem Danyal-Faheem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested these changes on the latest redwood release, and it seems to be working just fine for me. I've even tested my own changes in #1065 with this PR and it seems to be working.

However, I think the mysql-native-password option should be removed as it seemed to be working fine for me even after removing it.

@DawoudSheraz DawoudSheraz changed the title Supply mysql8.4.0 be default Supply mysql8.4.0 by default May 31, 2024
@rohan-saeed
Copy link
Contributor Author

Changes have been tested on dev environment as well as k8s. The mysql-native-password has been removed as it's working fine without it.

Copy link
Contributor

@DawoudSheraz DawoudSheraz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Played around a bit locally in dev, things seem to be working. 👍🏽

Copy link
Contributor

@Danyal-Faheem Danyal-Faheem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@regisb
Copy link
Contributor

regisb commented Jun 5, 2024

@ormsbee did you see this? Would you agree that we upgrade MySQL to 8.4.0 in Redwood, given that 8.1.0 is EOL?

@ormsbee
Copy link
Contributor

ormsbee commented Jun 5, 2024

@regisb: I had not seen it, thank you for tagging me. I agree that it makes sense to go to 8.4 in Redwood.

@regisb
Copy link
Contributor

regisb commented Jun 6, 2024

FTR I expect we'll stick with 8.4.0 for as long as possible (2029).

@DawoudSheraz
Copy link
Contributor

@regisb Thanks for the discussions on this. Once you give a 👍🏽 , I will merge this.

Copy link
Contributor

@regisb regisb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be good to go once the changelog entry is updated. We should do some hand-holding here :)

@regisb
Copy link
Contributor

regisb commented Jun 7, 2024

Looking good :) please make sure to squash all commits during rebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

5 participants