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

Chore/pimcore template #28

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

r-vanstraaten
Copy link

Work in progress

<<: *php-env-vars
depends_on: [mysql]

mysql:

Choose a reason for hiding this comment

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

In the .env.dev file DB_HOST=db is specified. db is also more common used in docker-compose files than mysql (as the name then doesn't specify the type of database used)

volumes:
- db:/var/lib/mysql
environment:
DB_HOST: "${DB_HOST}"

Choose a reason for hiding this comment

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

DB_HOST isn't necessary within this container (it doesn't need to know it's own host name)

- db:/var/lib/mysql
environment:
DB_HOST: "${DB_HOST}"
DB_USER: "${DB_USER}"

Choose a reason for hiding this comment

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

See https://hub.docker.com/_/mysql section "Environment variables"
It expects environment vars with names "MYSQL_USER" etc, so it creates the right user + database on first boot

ports:
- "${MYSQL_PORT}:3306"

redis-alpine:

Choose a reason for hiding this comment

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

See magento template, we can use a volume here as well (so cached data remains stored)

DB_USER: ~
DB_PASS: ~
DB_NAME: ~
MYSQL_ROOT_PASSWORD: ~

Choose a reason for hiding this comment

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

MySQL root password shouldn't be necessary within the PHP containers. There is no need (actually discouraged) for your application to connect with the root user

- elasticsearch:/usr/share/elasticsearch/data
environment:
- discovery.type=single-node
- "ES_JAVA_OPTS=-Xms512m -Xmx512m"

Choose a reason for hiding this comment

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

Possibly have that 512M memory setting in an .env.dev so it can be configured/overriden locally (if you're on a crappy machine for example)

[
mysqld,
--character-set-server=utf8mb4,
--collation-server=utf8mb4_unicode_ci,

Choose a reason for hiding this comment

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

Is utf8mb4_unicode_520_ci supported (that one is a bit better)

Copy link
Author

Choose a reason for hiding this comment

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

As best I can tell by looking at mysql 8.x documentation it should be supported yes.
https://dev.mysql.com/doc/mysql-g11n-excerpt/8.0/en/charset-mysql.html

@r-vanstraaten r-vanstraaten force-pushed the chore/pimcore-template branch 3 times, most recently from f57fee5 to 9574639 Compare January 6, 2022 12:58
environment:
MYSQL_USER: "${DB_USER}"
MYSQL_PASSWORD: "${DB_PASS}"
MYSQL_DATABASE: "${DB_NAME}"

Choose a reason for hiding this comment

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

db_1 | 2022-01-06 16:22:16+00:00 [Note] [Entrypoint]: Entrypoint script for MySQL Server 8.0.27-1debian10 started.
db_1 | 2022-01-06 16:22:16+00:00 [ERROR] [Entrypoint]: Database is uninitialized and password option is not specified
db_1 | You need to specify one of the following:
db_1 | - MYSQL_ROOT_PASSWORD
db_1 | - MYSQL_ALLOW_EMPTY_PASSWORD
db_1 | - MYSQL_RANDOM_ROOT_PASSWORD

Copy link
Author

Choose a reason for hiding this comment

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

Whoops, removed it here as well apparently when correcting your comment #28 (comment), added it back

@r-vanstraaten r-vanstraaten force-pushed the chore/pimcore-template branch from 9574639 to ed77e9a Compare January 7, 2022 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants