-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[WebServerBundle] added a way to dump current status host/port/address when getting the status #22316
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
Conversation
Here is how I'm using this in my Makefile:
|
…s when getting the status
cf724dd
to
b417b62
Compare
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.
Clever!
@@ -35,6 +35,7 @@ protected function configure() | |||
->setName('server:status') | |||
->setDefinition(array( | |||
new InputOption('pidfile', null, InputOption::VALUE_REQUIRED, 'PID file'), | |||
new InputOption('filter', null, InputOption::VALUE_REQUIRED, 'The value to display (one of port, host, or address)'), |
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.
What about something like --value | --get | --get-value
?
if ($server->isRunning($input->getOption('pidfile'))) { | ||
list($host, $port) = explode(':', $address = $server->getAddress($input->getOption('pidfile'))); | ||
if ('address' === $filter) { | ||
$output->write($address); |
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.
Should we use OUTPUT_RAW
to avoid formatting possible malicious input?
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.
input comes from the pidfile. If you mess up with it, then you are on your own anyway.
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.
Note it can also be messed up by some formatter implementation. But fair enough.. what are the odds anyway :)
…ost/port/address when getting the status (fabpot) This PR was merged into the 3.3-dev branch. Discussion ---------- [WebServerBundle] added a way to dump current status host/port/address when getting the status | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a Now that the web server can run on an automatically chosen port, it's more complex to integrate it into tests (I want to run some functional tests locally). The `server:status` command gives this information, but not in a machine-readable way. I propose to add a `--filter` flag to fix that: `./bin/console server:status --filter=address` Commits ------- b417b62 [WebServerBundle] added a way to dump current status host/port/address when getting the status
Now that the web server can run on an automatically chosen port, it's more complex to integrate it into tests (I want to run some functional tests locally).
The
server:status
command gives this information, but not in a machine-readable way.I propose to add a
--filter
flag to fix that:./bin/console server:status --filter=address