Web server bundle #21039

fabpot · 2016-12-23T21:49:03Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	yes
Deprecations?	no
Tests pass?	yes
Fixed tickets	#21040
License	MIT
Doc PR	n/a

Moved the server:* commands to a new bundle. It makes them more easily discoverable and more decoupled. Discoverability is important when not using symfony/symfony. In that case, the commands are not available unless you have the symfony/process component installed. With a dedicated bundle, installing the bundle also installs the dependency, making the whole process easier.

Usage is the same as the current commands for basic usage:

To start a web server in the foreground:

bin/console server:run

To manage a background server:

bin/console server:start
bin/console server:stop
bin/console server:status

The big difference is that port is auto-determined if something is already listening on port 8000.

Usage is different if you pass options:

bin/console server:start 127.0.0.1:8888
bin/console server:stop # no need to pass the address again
bin/console server:status # no need to pass the address again

That's possible as the web server now stores its address in a pid file stored in the current directory.

javiereguiluz · 2016-12-24T12:30:04Z

src/Symfony/Bundle/WebServerBundle/Command/ServerRunCommand.php

+    {
+        $this
+            ->setDefinition(array(
+                new InputArgument('addressport', InputArgument::OPTIONAL, 'The address to listen to (can be address:port, address, or port)', '127.0.0.1:8000'),


Minor: instead of addressport, we could call this uri because we accept schemes or not, IP address or hostnames, port numbers or not, etc. So this looks like a URI.

I think I would name this socket.

What about address? Like WebServer does?

stof · 2016-12-24T13:37:43Z

src/Symfony/Bundle/WebServerBundle/Command/ServerRunCommand.php

+
+        $callback = null;
+        $disableOutput = false;
+        if (OutputInterface::VERBOSITY_NORMAL > $output->getVerbosity()) {


this means === QUIET, right ? If yes, it may be much more readable

->isQuiet() ;)

stof · 2016-12-24T13:40:26Z

src/Symfony/Bundle/WebServerBundle/Command/ServerStartCommand.php

+                'You can either install it or use the "server:run" command instead.',
+            ));
+
+            if ($io->ask('Do you want to execute <info>server:run</info> immediately? [Yn] ', true)) {


by making true the default, it means that a non-interactive process will automatically fallback to server:run. Is it expected ? I don't think it is a good idea, as it switches to a long-lived command. I think this should be done only for interactive inputs (returning the failure exit code otherwise)

@fabpot what about this ?

It is like this on the current commands. So, I'm not sure.

ok, I've made a new commit to change the default value to false

stof · 2016-12-24T13:42:51Z

src/Symfony/Bundle/WebServerBundle/Command/ServerStatusCommand.php

            ->setDefinition(array(
-                new InputArgument('address', InputArgument::OPTIONAL, 'Address:port', '127.0.0.1:8000'),
-                new InputOption('port', 'p', InputOption::VALUE_REQUIRED, 'Address port number', '8000'),


removing this option is a BC break. We should deprecate it instead (even though the class is moved elsewhere, it is the same API for people using the CLI)

Introducing a new composer package with a deprecation by default, is probably a bad idea? It should be done from the framework bundle.

We never kept BC on CLI commands AFAIK.

stof · 2016-12-24T13:46:40Z

src/Symfony/Bundle/WebServerBundle/WebServer.php

+            }
+        }
+
+        return false;


I prefer a string|null API than string|false. The absence of value is null, not a boolean

stof · 2016-12-24T13:48:31Z

src/Symfony/Bundle/WebServerBundle/WebServer.php

+        $this->env = $env;
+    }
+
+    public function run($router = null, $disableOutput = true, $callback = null)


should it be typehinted as callable ?

stof · 2016-12-24T13:49:49Z

src/Symfony/Bundle/WebServerBundle/composer.json

@@ -0,0 +1,35 @@
+{
+    "name": "symfony/web-server-bundle",


you forgot the replace rule for this package in the root

stof · 2016-12-24T13:50:46Z

src/Symfony/Bundle/WebServerBundle/composer.json

+    "name": "symfony/web-server-bundle",
+    "type": "web-server-bundle",
+    "description": "Symfony WebServerBundle",
+    "keywords": [],


It would be good to add some keywords for discoverability through the Packagist search

mickaelandrieu

Great move, with hope it's a start for a more readable console in full edition.

2 questions : will this bundle be installed by default on Symfony 4?

How about a Symfony/web-server package and a bundle? A lot of frameworks and cmses can be interested to reuse this tiny but useful piece of code.

Merry Christmas Fabien :)

ro0NL · 2016-12-26T22:12:59Z

I would not need really the commands by default, i guess.

What about moving WebServer to symfony/process. As it already uses that, there's no real dependency issue or so. PhpServerProcess?

sstok · 2016-12-27T10:07:24Z

src/Symfony/Bundle/WebServerBundle/WebServer.php

+    public function __construct($address = '127.0.0.1:8000')
+    {
+        if (false !== strpos($address, ':')) {
+            list($this->hostname, $this->port) = explode(':', $address);


This will not work when IPv6 is used.

The format for IPv6 should actually be [address]:port, eg. [::1]:8000

fabpot · 2017-01-04T19:08:25Z

Auto-port detection has just been added. I've also updated the description to describe usage and differences with current commands.

fabpot · 2017-01-05T17:30:04Z

ping @symfony/deciders

nicolas-grekas · 2017-01-05T18:35:02Z

👍

mickaelandrieu · 2017-01-05T18:42:28Z

Also, as the behavior have changed this need - at least - an issue to document it :)

javiereguiluz · 2017-01-05T19:08:12Z

src/Symfony/Bundle/WebServerBundle/Command/ServerStatusCommand.php

+        $io = new SymfonyStyle($input, $output);
+        $server = new WebServer();
+        if ($server->isRunning($input->getOption('pidfile'))) {
+            $io->success('Web server still listening.');


Is it possible to show the address and port where the port is listening? Otherwise this command doesn't provide much help:

stof · 2017-01-05T18:54:39Z

src/Symfony/Bundle/WebServerBundle/Command/ServerStartCommand.php

+                new InputArgument('addressport', InputArgument::OPTIONAL, 'The address to listen to (can be address:port, address, or port)', null),
+                new InputOption('docroot', 'd', InputOption::VALUE_REQUIRED, 'Document root', null),
+                new InputOption('router', 'r', InputOption::VALUE_REQUIRED, 'Path to custom router script'),
+                new InputOption('pidfile', '', InputOption::VALUE_REQUIRED, 'PID file', null),


second argument should be null

fixed everywhere

stof · 2017-01-05T19:13:30Z

src/Symfony/Bundle/WebServerBundle/Command/ServerStartCommand.php

+                new InputArgument('addressport', InputArgument::OPTIONAL, 'The address to listen to (can be address:port, address, or port)', null),
+                new InputOption('docroot', 'd', InputOption::VALUE_REQUIRED, 'Document root', null),
+                new InputOption('router', 'r', InputOption::VALUE_REQUIRED, 'Path to custom router script'),
+                new InputOption('pidfile', '', InputOption::VALUE_REQUIRED, 'PID file', null),


The help of the comment should explain the PID file IMO

stof · 2017-01-05T19:14:02Z

src/Symfony/Bundle/WebServerBundle/LICENSE

@@ -0,0 +1,19 @@
+Copyright (c) 2004-2016 Fabien Potencier


stof · 2017-01-05T19:16:47Z