-
Notifications
You must be signed in to change notification settings - Fork 2.2k
docs: update readme from mariadb with port binding, tags, remove double spaces and informations #2552
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
base: master
Are you sure you want to change the base?
docs: update readme from mariadb with port binding, tags, remove double spaces and informations #2552
Conversation
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.
I've left a few notes 👍
I'll also explicitly CC the mariadb
image maintainers: @grooverdan @dbart @fauust
mariadb/stack.yml
Outdated
image: mariadb:11.7-ubi | ||
restart: always | ||
environment: | ||
MARIADB_ROOT_PASSWORD: example | ||
ports: | ||
- 3306:3306 | ||
|
||
adminer: | ||
image: adminer | ||
image: adminer:4.17.1 |
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.
These really specific version numbers are likely to bitrot quickly, so we don't typically suggest being so particular unless it's actually necessary to be.
(Also, see the prior note about exposed database ports.)
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.
especially with 11.7 as a short term support release. Perhaps use a lts
/lts-ubi
tag and I guess something for adminer. The purpose of compose files was self contained, so especially with something link adminer, exposing ports seems unnecessary.
On env variables including something like MARIADB_AUTO_UPGRADE=1
would smooth the upgrade paths.
I wouldn't mind seeing the healthcheck and service dependency like https://mariadb.com/kb/en/using-healthcheck-sh/#compose-file-example here as a practically of showing of getting the database up before the services that connect to it (and might fail if they can't connect). (edit: assuming docker stack
compatibility).
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.
These really specific version numbers are likely to bitrot quickly, so we don't typically suggest being so particular unless it's actually necessary to be.
(Also, see the prior note about exposed database ports.)
updated.
especially with 11.7 as a short term support release. Perhaps use a
lts
/lts-ubi
tag and I guess something for adminer. The purpose of compose files was self contained, so especially with something link adminer, exposing ports seems unnecessary.On env variables including something like
MARIADB_AUTO_UPGRADE=1
would smooth the upgrade paths.I wouldn't mind seeing the healthcheck and service dependency like https://mariadb.com/kb/en/using-healthcheck-sh/#compose-file-example here as a practically of showing of getting the database up before the services that connect to it (and might fail if they can't connect). (edit: assuming
docker stack
compatibility).
I like the idea.
I think another section, with this more complete example, would be better. Then we can have a more basic section and a more advanced one.
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 thread isn't resolved (references in stack.yml
are still too-specific).
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.
thanks for the cleanup.
@tianon, @grooverdan |
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.
Do I need to make any further adjustments for analysis and approval?
sorry for delay. I'll get to it next week. |
@grooverdan |
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.
previous comments remain unaddressed.
|
||
services: | ||
|
||
db: | ||
image: mariadb | ||
image: mariadb:11.7 |
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.
there's still specific versions here.
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.
MARIADB_AUTO_UPGRADE=1
isn't there in env
restart: always | ||
environment: | ||
MARIADB_ROOT_PASSWORD: example | ||
|
||
adminer: | ||
image: adminer | ||
image: adminer:5.0.1 |
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.
while I do generally like the idea of pinning versions, on a UI is it important? With this versioned pinned the updates to this are going to be quite frequent - current adminer versions: https://github.com/docker-library/official-images/blob/master/library/adminer
This pull request includes updates to the
mariadb
documentation and configuration files to improve clarity and compatibility, as well as to specify versions for certain images.Documentation improvements:
mariadb/content.md
: Added an important note about the%%IMAGE%%
tags to highlight thelatest
andlts
versions.mariadb/content.md
: Updated the Docker compose command to usedocker compose
instead ofdocker-compose
for consistency with the latest Docker CLI.mariadb/content.md
: Added a section on port binding to explain how to expose the container port to the host port using the-p
argument in thedocker run
command.Configuration updates:
mariadb/stack.yml
: Specified the image versions formariadb
andadminer
to ensure consistency and compatibility. Themariadb
image is updated to11.7-ubi
andadminer
to4.17.1
.mariadb/stack.yml
: Remove docker compose version on file header - the docker compose version is obsolete