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

Move from MYSQL_DISTRIBUTION -> DB_DISTRIBUTION #665

Closed
wants to merge 3 commits into from
Closed

Conversation

navarr
Copy link
Member

@navarr navarr commented Jun 19, 2023

Part of ensuring bc-compatibility with Den

@navarr navarr added the enhancement New feature or request label Jun 19, 2023
environments/magento1/init.env Outdated Show resolved Hide resolved
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"}}}
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. shoudln't we add export there?
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"}}}
  1. 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?

Copy link
Member Author

@navarr navarr Jul 7, 2023

Choose a reason for hiding this comment

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

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

@navarr navarr linked an issue Aug 28, 2023 that may be closed by this pull request
@navarr navarr added this to the Warden 0.15 milestone Jan 3, 2024
Copy link

@rootindex rootindex left a comment

Choose a reason for hiding this comment

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

Removing the extra bracket, solved my issues

@@ -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}}

Choose a reason for hiding this comment

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

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}}

Choose a reason for hiding this comment

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

Extra bracket causes build issues.

@@ -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}}

Choose a reason for hiding this comment

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

Extra bracket causes build issues.

@@ -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}}

Choose a reason for hiding this comment

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

Extra bracket causes build issues.

@navarr navarr closed this Sep 23, 2024
@ihor-sviziev
Copy link
Contributor

@navarr just wonder, why you closed it?

@navarr
Copy link
Member Author

navarr commented Sep 24, 2024

@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
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Image] magento 2 error pull maria db
4 participants