-
-
Notifications
You must be signed in to change notification settings - Fork 874
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
Support MySQL 8.4 #549
base: master
Are you sure you want to change the base?
Support MySQL 8.4 #549
Conversation
@@ -61,7 +73,7 @@ | |||
with_items: "{{ mysql_root_hosts.stdout_lines|default([]) }}" | |||
when: > | |||
((mysql_install_packages | bool) or mysql_root_password_update) | |||
and ('5.7.' not in mysql_cli_version.stdout and '8.0.' not in mysql_cli_version.stdout) | |||
and ('5.7.' not in mysql_cli_version.stdout and '8.' not in mysql_cli_version.stdout) |
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.
The only downside to this change is maybe the comparison would break if there were a version like 5.8.0
, that version would be identified as 8.
... trying to think of a better way to narrow it. Maybe add in something to trim off the first two digits, and compare that to 8.
?
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 was thinking about this solution, but as far as I know, version 5.8 doesn't exist. However, everything works fine on version >= 5.7. If you want, I can change the version definition using trim.
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 think using trim would be ever so slightly more future proof. It feels more correct to me.
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.
Hi Guys, thanks for your awsome work. i would appreciate if this got released in near future :) I´ve successully tested with mysql 8.4 and ubuntu 24.04. Let me know if i can assist. Cheers Tim |
We also need to add |
@@ -61,7 +73,7 @@ | |||
with_items: "{{ mysql_root_hosts.stdout_lines|default([]) }}" | |||
when: > | |||
((mysql_install_packages | bool) or mysql_root_password_update) | |||
and ('5.7.' not in mysql_cli_version.stdout and '8.0.' not in mysql_cli_version.stdout) | |||
and ('5.7.' not in mysql_cli_version.stdout and '8.' not in mysql_cli_version.stdout) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
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.
@geerlingguy could you please give some attention to the alternative presented here?
I need to ensure support for MySQL 8.4 and at the moment, despite installing the service, the configuration is incorrect and the service does not start.
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.
It looks good — can you maybe file a PR based on this including your changes, and I can merge that once it's passing tests?
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.
@geerlingguy I think 8.4 support was implemented in PR https://github.com/geerlingguy/ansible-role-mysql/pull/561/files
Could you please validate?
@@ -104,7 +104,7 @@ lower_case_table_names = {{ mysql_lower_case_table_names }} | |||
event_scheduler = {{ mysql_event_scheduler_state }} | |||
|
|||
# InnoDB settings. | |||
{% if mysql_supports_innodb_large_prefix and '8.0.' not in mysql_cli_version.stdout %} | |||
{% if mysql_supports_innodb_large_prefix and '8.' not in mysql_cli_version.stdout %} |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
@@ -87,7 +87,7 @@ read_buffer_size = {{ mysql_read_buffer_size }} | |||
read_rnd_buffer_size = {{ mysql_read_rnd_buffer_size }} | |||
myisam_sort_buffer_size = {{ mysql_myisam_sort_buffer_size }} | |||
thread_cache_size = {{ mysql_thread_cache_size }} | |||
{% if '8.0.' not in mysql_cli_version.stdout %} | |||
{% if '8.' not in mysql_cli_version.stdout %} |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
@@ -50,7 +62,7 @@ | |||
with_items: "{{ mysql_root_hosts.stdout_lines|default([]) }}" | |||
when: > | |||
((mysql_install_packages | bool) or mysql_root_password_update) | |||
and ('5.7.' in mysql_cli_version.stdout or '8.0.' in mysql_cli_version.stdout) | |||
and ('5.7.' in mysql_cli_version.stdout) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
with_items: "{{ mysql_root_hosts.stdout_lines|default([]) }}" | ||
when: > | ||
((mysql_install_packages | bool) or mysql_root_password_update) | ||
and ('8.' in mysql_cli_version.stdout) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
I've changed the method for resetting the root user password in MySQL 8+ since
mysql_native_password
is no longer enabled by default. Link https://dev.mysql.com/doc/refman/8.4/en/native-pluggable-authentication.htmlNow, versions of MySQL 8.* are correctly identified.