-
-
Notifications
You must be signed in to change notification settings - Fork 580
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
fix: check for valid db version in ddev debug migrate-database
#6168
fix: check for valid db version in ddev debug migrate-database
#6168
Conversation
Download the artifacts for this pull request:
See Testing a PR. |
@@ -207,7 +209,7 @@ func GetValidMariaDBVersions() []string { | |||
// IsValidPostgresVersion is a helper function to determine if a PostgreSQL version is valid, returning | |||
// true if the supplied version is valid and false otherwise. | |||
func IsValidPostgresVersion(v string) bool { | |||
if _, ok := ValidPostgresVersions[v]; !ok { | |||
if _, ok := ValidPostgresVersions[strings.TrimPrefix(v, Postgres+":")]; !ok { |
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.
Should this be Postgres
or postgres
?
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.
Postgres
, this is a constant:
Lines 17 to 22 in 195334d
// Database Types | |
const ( | |
MariaDB = "mariadb" | |
MySQL = "mysql" | |
Postgres = "postgres" | |
) |
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.
This works perfectly and looks right. I just didn't understand the validator change.. was it always wrong???
func IsValidMariaDBVersion(MariaDBVersion string) bool { | ||
if _, ok := ValidMariaDBVersions[MariaDBVersion]; !ok { | ||
func IsValidMariaDBVersion(v string) bool { | ||
if _, ok := ValidMariaDBVersions[strings.TrimPrefix(v, MariaDB+":")]; !ok { |
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.
So these were only used in ddev debug migrate-database
and were always 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.
Yes, that's right.
I didn't even know the validator was broken, the condition in ddev debug migrate-database
looked strange and I investigated it.
The Issue
How This PR Solves The Issue
Fixed the condition for valid db versions. I came across it and couldn't understand how it worked before. During debugging, I saw that instead of the version, for example,
8.0
, the whole stringmysql:8.0
was passed inside.Manual Testing Instructions
Automated Testing Overview
Related Issue Link(s)
Release/Deployment Notes