Move from MYSQL_DISTRIBUTION -> DB_DISTRIBUTION #665
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bap14
reviewed
Jun 19, 2023
navarr
commented
Jul 6, 2023
ihor-sviziev
suggested changes
Jul 7, 2023
commands/env.cmd
Outdated
Comment on lines
12
to
13
| DB_DISTRIBUTION=${DB_DISTRIBUTION:-${MYSQL_DISTRIBUTION:-"mariadb"}} | ||
| DB_DISTRIBUTION_VERSION=${DB_DISTRIBUTION_VERSION:-${MYSQL_DISTRIBUTION_VERSION:-${MARIADB_VERSION:-"10.4"}}} |
There was a problem hiding this comment.
- shoudln't we add
exportthere?
Suggested change
| DB_DISTRIBUTION=${DB_DISTRIBUTION:-${MYSQL_DISTRIBUTION:-"mariadb"}} | |
| DB_DISTRIBUTION_VERSION=${DB_DISTRIBUTION_VERSION:-${MYSQL_DISTRIBUTION_VERSION:-${MARIADB_VERSION:-"10.4"}}} | |
| export DB_DISTRIBUTION=${DB_DISTRIBUTION:-${MYSQL_DISTRIBUTION:-"mariadb"}} | |
| export DB_DISTRIBUTION_VERSION=${DB_DISTRIBUTION_VERSION:-${MYSQL_DISTRIBUTION_VERSION:-${MARIADB_VERSION:-"10.4"}}} |
- maybe it's better to pring some warning if the DB_DISTRIBUTION and DB_DISTRIBUTION_VERSION variable aren't declared yet? In this case we might suggest changing MYSQL_DISTRIBUTION variable to DB_DISTRIBUTION and MYSQL_DISTRIBUTION_VERSION to DB_DISTRIBUTION_VERSION. What do you think?
There was a problem hiding this comment.
I don't exactly follow point 2.
Warden's old default is MariaDB 10.4. A few versions ago I mistakenly introduced the ability to change database distribution and version using MYSQL_ variables (the mistake)
Moving to DB_DISTRIBUTION so it matches Den is what I'm trying to do here, while maintaining backwards compatibility with any .env file that uses MYSQL_ variables, and maintaining backwards compatibility with the original defaults
Co-authored-by: Ihor Sviziev <ihor-sviziev@users.noreply.github.com>
ihor-sviziev
approved these changes
Jul 7, 2023
rootindex
suggested changes
Apr 5, 2024
| @@ -6,7 +6,7 @@ services: | |||
|
|
|||
| db: | |||
| hostname: "${WARDEN_ENV_NAME}-mariadb" | |||
| image: ${WARDEN_IMAGE_REPOSITORY}/${MYSQL_DISTRIBUTION:-mariadb}:${MYSQL_DISTRIBUTION_VERSION:-${MARIADB_VERSION:-10.4}} | |||
| image: ${WARDEN_IMAGE_REPOSITORY}/${DB_DISTRIBUTION}:${DB_DISTRIBUTION_VERSION}} | |||
There was a problem hiding this comment.
Extra bracket causes build issues.
image: ${WARDEN_IMAGE_REPOSITORY}/${DB_DISTRIBUTION}:${DB_DISTRIBUTION_VERSION}
| @@ -10,7 +10,7 @@ services: | |||
|
|
|||
| checkoutdb: | |||
| hostname: "${WARDEN_ENV_NAME}-checkoutdb" | |||
| image: ${WARDEN_IMAGE_REPOSITORY}/${MYSQL_DISTRIBUTION:-mariadb}:${MYSQL_DISTRIBUTION_VERSION:-${MARIADB_VERSION:-10.4}} | |||
| image: ${WARDEN_IMAGE_REPOSITORY}/${DB_DISTRIBUTION}:${DB_DISTRIBUTION_VERSION}} | |||
| @@ -10,7 +10,7 @@ services: | |||
|
|
|||
| salesdb: | |||
| hostname: "${WARDEN_ENV_NAME}-salesdb" | |||
| image: ${WARDEN_IMAGE_REPOSITORY}/${MYSQL_DISTRIBUTION:-mariadb}:${MYSQL_DISTRIBUTION_VERSION:-${MARIADB_VERSION:-10.4}} | |||
| image: ${WARDEN_IMAGE_REPOSITORY}/${DB_DISTRIBUTION}:${DB_DISTRIBUTION_VERSION}} | |||
| @@ -2,7 +2,7 @@ version: "3.5" | |||
| services: | |||
| tmp-mysql: | |||
| hostname: "${WARDEN_ENV_NAME}-mysql" | |||
| image: ${WARDEN_IMAGE_REPOSITORY}/${MYSQL_DISTRIBUTION:-mariadb}:${MYSQL_DISTRIBUTION_VERSION:-${MARIADB_VERSION:-10.4}} | |||
| image: ${WARDEN_IMAGE_REPOSITORY}/${DB_DISTRIBUTION}:${DB_DISTRIBUTION_VERSION}} | |||
|
@navarr just wonder, why you closed it? |
|
@ihor-sviziev it doesn't feel like there's a point in the backwards compatibility after all this time |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of ensuring bc-compatibility with Den