wp-env: Add phpMyAdmin support #67588
Merged
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
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: 0 B Total Size: 1.83 MB ℹ️ View Unchanged
|
|
Any reason for this to be an option and not something that comes by default? |
|
Flaky tests detected in 95b6649. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12239409252
|
I started out by just making it on by default. Seeing the I suppose the only question for me is this: is there any scenario in which a developer may not want to have the database exposed via phpMyAdmin on their local machine, even though they are using |
|
You know what, what bothers me the most is not the fact that these are options, but the fact that they are CLI flags. We already have a config file |
Right now in this PR you can use the config file to provide ports, but not to enable it. But I think that's a good point. So is this the UX you'd like to see? {
"$schema": "./schemas/json/wp-env.json",
"core": "WordPress/WordPress",
"plugins": [ "." ],
"env": {
"development": {
"phpmyadmin": 9000
},^ where this config automatically enables the phpMyAdmin service. Correct? I think that's alright. If I have to look for drawbacks, I would say that the CLI options have the benefit of easier discovery: anyone invoking |
Yes, although I don't like the env subkeys, I think we should get rid of the test+dev env setup at some point, if you want two environments, you just have two config files and trigger two environments. But this is a separate discussion :)
I value consistency, why CLI args just for these particular configs and not all the other ones. |
mcsf
force-pushed
the
add/wp-env-phpmyadmin
branch
from
e590564 to
fc7cad4
Compare
|
I've removed the |
|
Meanwhile, just to document my own decision: A single phpMyAdmin Docker image is capable of connecting to multiple database hosts. I considered using that to solve the problem of connecting to both the All in all, it's not a huge difference, but the 2-services approach won. |
|
This is looking good to me, I'll test it better a bit later. Also, is it enabled by default on some default port or should we actually have to define the port for it to work? |
|
Can we add a small changelog entry as well. |
- Add option `--phpmyadmin` to `wp-env start` (off by default) - Defaults to port 9000 - Configurable in .wp-env.json under key `phpmyadminPort` - Overridden by environment variable `WP_ENV_PHPMYADMIN_PORT`
- Refactor repeating logic into getPublicDockerPort - Remove trailing newlines from dockerCompose output - Refactor evaluation of spinner.prefixText so as to clearly reveal newlines.
Two docker-compose services are now defined: `phpmyadmin` and
`tests-phpmyadmin`. These are off by default. They can be individually
turned on by specifying a port in the each enviroment's config:
{
env: {
development: {
...
phpmyadminPort: 9000
},
tests: {
...
}
}
}
mcsf
force-pushed
the
add/wp-env-phpmyadmin
branch
from
fc7cad4 to
50ea645
Compare
The port has to be explicitly defined to enable the service. I think we need a confident answer to this question before we can just turn it on by default:
I propose merging this PR with the opt-in behaviour and figuring it out separately. |
youknowriad
approved these changes
Dec 9, 2024
youknowriad
added
the
Needs Dev Note
ramonjd
added
[Type] Build Tooling
yogeshbhutkar
pushed a commit
to yogeshbhutkar/gutenberg
that referenced
this pull request
Dec 18, 2024
* wp-env: Add phpMyAdmin support
Defines two new docker-compose services: `phpmyadmin` and
`tests-phpmyadmin`. These are off by default. They can be individually
turned on by:
- Specifying a port in any enviroment's config via key `phpmyadminPort`
(see bottom of commit message for example).
- By setting environment variable `WP_ENV_PHPMYADMIN_PORT` and/or
`WP_ENV_TESTS_PHPMYADMIN_PORT`.
* Opt into the phpMyAdmin service in the Gutenberg environment
* wp-env: start: refactor computation of command output
- Refactor repeating logic into getPublicDockerPort
- Remove trailing newlines from dockerCompose output
- Refactor evaluation of spinner.prefixText so as to clearly reveal
newlines.
{
env: {
development: {
...
phpmyadminPort: 9000
},
tests: {
...
}
}
}
---------
Co-authored-by: Riad Benguella <[email protected]>
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.
What?
Adds phpMyAdmin as an optional service to wp-env.
Why?
Makes it easier for developers to inspect their wp-env databases, at very little cost.
How?
phpmyadminandtests-phpmyadmin.Adds a switch towp-env start(--phpmyadmin) to opt in to the service..wp-env.jsonenvironment config key (phpmyadminPort) and environment variables (WP_ENV_PHPMYADMIN_PORT,WP_ENV_TESTS_PHPMYADMIN_PORT).Testing Instructions
Run
WP_ENV_PHPMYADMIN_PORT=9000 npx wp-env start.Open
localhost:9000.Verify that phpMyAdmin is available and correctly connected to the
mysqldatabase service.Repeat, but, instead of passing
WP_ENV_PHPMYADMIN_PORT=9000, edit your.wp-env.jsonfile to add the following key-value pair underenv > developmentand just runnpx wp-env start:{ "env": { "development": { "phpmyadminPort": 9000WP_ENV_TESTS_PHPMYADMIN_PORT=9003 npx wp-env startand check that thetests-phpmyadminis correctly launched.env > testsand check that thetests-phpmyadminis correctly launched.Checklist
startinvocation:npx wp-env start --phpmyadmin=9000?