Skip to content

Warn if both WORDPRESS_DB_HOST and MYSQL_PORT_3306_TCP are set #57

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

Merged
merged 1 commit into from
Feb 9, 2015
Merged

Warn if both WORDPRESS_DB_HOST and MYSQL_PORT_3306_TCP are set #57

merged 1 commit into from
Feb 9, 2015

Conversation

md5
Copy link
Contributor

@md5 md5 commented Feb 7, 2015

No description provided.

@mrjackdavis
Copy link

LGTM, I feel this is getting a bit messy though

@md5
Copy link
Contributor Author

md5 commented Feb 9, 2015

I'm curious how you think it's getting messy.

The only messy part in my opinion is that some people had been relying on the undocumented ability to set MYSQL_PORT_3306_TCP to configure an external database for this image. Since that ability was not a supported feature, I think @tianon's change in 9aae243 was reasonable.

Now that the ability to configure an external db directly with WORDPRESS_DB_HOST has been added, the situation seems pretty clean to me:

  • If you want a linked database, use --link mysql and the mysql entry in /etc/hosts with be used as DB_HOST in wp-config.php
  • If you want to use an external database, set WORDPRESS_DB_HOST and that value will be used directly for DB_HOST in wp-config.php

Once this PR is merged (or something like it), the user will be warned if they try to use both -e WORDPRESS_DB_HOST=... and --link some-mysql:mysql. If someone had been using MYSQL_PORT_3306_TCP directly and are now wondering why Wordpress isn't trying to connect to their external db, at least those people will also get an error message now.

@mrjackdavis
Copy link

Sorry @md5, I should've provided a bit more detail.

The reason I feel it's getting messy (100% subjective), is that a good amount of fringe case checks are entering the docker-entrypoint.sh; making it a little hard to read. It's fine for now. But IMHO if we continue on the current track it'll get a little unmanageable.

Otherwise, the fix is great. Thanks for doing that

@tianon
Copy link
Member

tianon commented Feb 9, 2015

LGTM

@tianon
Copy link
Member

tianon commented Feb 9, 2015

cc @yosifkit

@yosifkit
Copy link
Member

yosifkit commented Feb 9, 2015

LGTM

yosifkit added a commit that referenced this pull request Feb 9, 2015
Warn if both WORDPRESS_DB_HOST and MYSQL_PORT_3306_TCP are set
@yosifkit yosifkit merged commit 37715c4 into docker-library:master Feb 9, 2015
tianon added a commit to infosiftr/stackbrew that referenced this pull request Feb 11, 2015
- `docker-dev`: 1.5.0
- `elasticsearch`: 1.3.8 and 1.4.3
- `mongo`: 3.0.0-rc8
- `rabbitmq`: 3.4.4; updated Erlang (docker-library/rabbitmq#9)
- `wordpress`: warn if both `WORDPRESS_DB_HOST` and linked MySQL are supplied (docker-library/wordpress#57)
@md5 md5 deleted the external-db-error-checking branch February 14, 2015 17:12
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.

4 participants