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.

Sign up for GitHub

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

Jump to bottom

Include syntax for setting mysql native password for mysql v8.4 #1568

Merged
merged 6 commits into from
Jun 20, 2024
Merged

Include syntax for setting mysql native password for mysql v8.4 #1568

merged 6 commits into from
Jun 20, 2024

Conversation

KTanAug21
Copy link
Contributor

@KTanAug21 KTanAug21 commented Jun 11, 2024 •
edited
Loading

Summary of changes

Update the mysql doc page to include v8.4 syntax for setting mysql-native password on

Preview

Related Fly.io community and GitHub links

https://fly.io/docs/app-guides/mysql-on-fly/

The update + debug issue section can help fix the errors reported in this community post. Namely the following errors:

  1. unknown option '--default-authentication-plugin'.
  2. 'mysql.user' doesn't exist

Notes

Kathryn Anne S Tan added 4 commits June 11, 2024 21:50
@KTanAug21 KTanAug21 marked this pull request as ready for review June 17, 2024 14:41
andie787
andie787 reviewed Jun 17, 2024
View reviewed changes
Copy link
Contributor

@andie787 andie787 left a comment

Choose a reason for hiding this comment

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

Left some suggestions to consider for formatting and showing the different configs for different version.

app-guides/mysql-on-fly.html.markerb Outdated
@@ -77,7 +75,8 @@ primary_region = "bos"
cpus = 1
memory_mb = 2048

# Add the following sections
# Add the following sections for 8+,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might be confusing, since you still add [mounts] and [env] for 5.7, don't you?

app-guides/mysql-on-fly.html.markerb Outdated
@@ -77,7 +75,8 @@ primary_region = "bos"
cpus = 1
memory_mb = 2048

# Add the following sections
# Add the following sections for 8+,
# Note that the [processes] section would be different for 8.4, and 5.7 ( the syntax will be shown below )
Copy link
Contributor

Choose a reason for hiding this comment

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

What if we put all three in the toml file, with 2 of them commented out? Something like:

Suggested change
# Note that the [processes] section would be different for 8.4, and 5.7 ( the syntax will be shown below )
# The [processes] section is different for 8+, 8.4, and 5.7. Use the one that matches your version.
# Use the following for versions 8 to 8.3:
[processes]
app = """--datadir /data/mysql \
--default-authentication-plugin mysql_native_password"""
# Uncomment and use the following for 8.4:
# [processes]
# app = """--datadir /data/mysql \
# --mysql-native-password=ON"""
# Uncomment and use the following for 5.7:
# [processes]
# app = "--datadir /data/mysql"
# Add the following sections for all versions
[mounts]
source = "mysqldata"
destination = "/data"
[env]
MYSQL_DATABASE = "some_db"
MYSQL_USER = "non_root_user"```

app-guides/mysql-on-fly.html.markerb Outdated
[processes]
app = """--datadir /data/mysql \
--mysql-native-password=ON"""
```
* **Important**: For MySQL 5.7 and MySQL 8+, we set MySQL's data directory to a subdirectory of our mounted volume.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can remove this, and just augment the "important" callout below.

app-guides/mysql-on-fly.html.markerb Outdated
* For MySQL 8+, you'll want to use the `mysql_native_password` password plugin.
* For MySQL 8+, you'll want to use the `mysql_native_password` authentication plugin

Starting from MySQL 8.4, the `--default-authentication-plugin` above has been [depreciated](https://github.com/docker-library/mysql/issues/1048#issuecomment-2088836076). If you are using this verion, please make sure to revise the [process] section to use an updated syntax of setting mysql-native-password:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Starting from MySQL 8.4, the `--default-authentication-plugin` above has been [depreciated](https://github.com/docker-library/mysql/issues/1048#issuecomment-2088836076). If you are using this verion, please make sure to revise the [process] section to use an updated syntax of setting mysql-native-password:
* Starting from MySQL 8.4, the `--default-authentication-plugin` option is [deprecated](https://github.com/docker-library/mysql/issues/1048#issuecomment-2088836076). If you're using 8.4+, make sure to revise the [process] section to use an updated syntax of setting mysql-native-password.

app-guides/mysql-on-fly.html.markerb Outdated
Comment on lines 102 to 106
```toml
[processes]
app = """--datadir /data/mysql \
--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.

Don't need this here if we put it into the fly.toml above.

app-guides/mysql-on-fly.html.markerb Outdated
app = "--datadir /data/mysql"
```

If you're using MySQL 5.7, your `app` process can be simplified:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If you're using MySQL 5.7, your `app` process can be simplified:
* If you're using MySQL 5.7, your `app` process can be simplified as shown in the `fly.toml` example above.

app-guides/mysql-on-fly.html.markerb Outdated
Comment on lines 110 to 113
```toml
[processes]
app = "--datadir /data/mysql"
```
Copy link
Contributor

Choose a reason for hiding this comment

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

delete this if included in the fly.toml above

app-guides/mysql-on-fly.html.markerb
```toml
[processes]
app = "--datadir /data/mysql"
```

<div class="important icon">
Copy link
Contributor

Choose a reason for hiding this comment

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

Update this whole note:

Suggested change
<div class="important icon">
<div class="important icon">
**Important**: Set MySQL's data directory to a subdirectory of your mounted volume. Mounting a disk in Linux usually results in a `lost+found` directory being created. However, MySQL won't initialize into a data directory unless it's completely empty. That's why you need to use a subdirectory of the mounted location: `/data/mysql`.
</div>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @andie787 !! Thank you for your reviews! I've updated the PR to reflect the suggestions you've made, it's a lot readable now, thank you!

@KTanAug21 @andie787
Update app-guides/mysql-on-fly.html.markerb
6d4efe9 
Looks better as a sub heading!

Co-authored-by: Andrea Anderson <andrea@fly.io>
@KTanAug21 KTanAug21 merged commit c9a9c09 into main Jun 20, 2024
3 checks passed
@KTanAug21 KTanAug21 deleted the update_mysql_8_4 branch June 20, 2024 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andie787 andie787 andie787 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KTanAug21 @andie787