[Messenger] Add debug:messenger CLI command #26803

ro0NL · 2018-04-04T18:16:16Z

Q	A
Branch?	master
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#...
License	MIT
Doc PR	symfony/symfony-docs#...

Adds a debug:messenger CLI command to expose message classes that can be dispatched. Heavily inspired by debug:autowiring.

ogizanagi · 2018-04-04T18:20:25Z

One less PR to do on my list :)

The tactician debug command looks like this:

Tactician routing
=================

Bus: default
------------

 ----------------------------------------------------- ----------------- 
  Command                                               Handler Service  
 ----------------------------------------------------- ----------------- 
  League\Tactician\Bundle\Tests\Fake\FakeCommand        a.handler        
  League\Tactician\Bundle\Tests\Fake\OtherFakeCommand   b.handler        
 ----------------------------------------------------- -----------------

https://github.com/thephpleague/tactician-bundle/blob/master/tests/Integration/DebugCommandTest.php#L37-L48

Would be great to have messages <-> handlers too here.

ogizanagi · 2018-04-04T18:23:07Z

src/Symfony/Bundle/FrameworkBundle/Command/DebugMessengerCommand.php

+    {
+        $this
+            ->setDefinition(array(
+                new InputArgument('search', InputArgument::OPTIONAL, 'A search filter'),


Not convinced this is useful until we have more info to show about a message. A simple grep is enough for this for now.

ro0NL · 2018-04-04T18:27:39Z

A message can have multiple handlers .. 🤔

Also the intend was concerning end users who mostly care about $bus->dispatch($whatCanIPutHere).

ogizanagi · 2018-04-04T18:30:33Z

A message can have multiple handlers ..

Wasn't the case with tactician, but it's not a problem here. Just show all associated handlers.

Also the intend was concerning end users who mostly care about $bus->dispatch($whatCanIPutHere).

Would still be useful in this way :)

fabpot · 2018-04-06T05:30:44Z

src/Symfony/Bundle/FrameworkBundle/Command/DebugMessengerCommand.php

+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\FrameworkBundle\Command;


Should be moved to the component like the other Messenger commands.

ro0NL · 2018-04-08T06:25:27Z

Latest version

sroze · 2018-04-08T10:07:56Z

src/Symfony/Component/Messenger/Command/DebugMessengerCommand.php

+        $tableRows = array();
+        foreach ($this->mapping as $message => $handlers) {
+            $tableRows[] = array(sprintf('<fg=cyan>%s</fg=cyan>', $message));
+            foreach ($handlers as $handler) {


I think that having the list of handlers in another column would make more sense and be more coherent with the rest of the debug: commands as well.

well.. i took debug:autowring for inspiration. IMHO that one reads well :)

Also with multiple columns we probably run into a spacing issue, assuming all service IDs are FQCN.

sroze · 2018-04-08T10:08:10Z

src/Symfony/Component/Messenger/Command/DebugMessengerCommand.php

+        $io = new SymfonyStyle($input, $output);
+
+        $io->title('Message Bus');
+        $io->text('The following classes can be dispatch:');


The following messages can be dispatched:

sroze · 2018-04-08T10:08:18Z

src/Symfony/Component/Messenger/Command/DebugMessengerCommand.php

+    {
+        $io = new SymfonyStyle($input, $output);
+
+        $io->title('Message Bus');


weaverryan · 2018-04-21T18:31:35Z

I do like the table layout mentioned above #26803 (comment) - it's just a bit easier to read.

I realize that it's possible for a message to have several handlers. This is an edge-case, and I think we could handle that like this:

 ----------------------------------------------------- ----------------- 
  Command                                               Handler Service  
 ----------------------------------------------------- ----------------- 
  League\Tactician\Bundle\Tests\Fake\FakeCommand        a.handler        
                                                        c.handler        
  League\Tactician\Bundle\Tests\Fake\OtherFakeCommand   b.handler        
 ----------------------------------------------------- -----------------

sroze · 2018-04-25T16:42:12Z

This PR needs to be updated following #26864 :)

ro0NL · 2018-04-28T07:40:19Z

Rebased.

@sroze we cant group messages per bus as there's no such concept, so it's still a single list of messages.

@weaverryan can you update the table example with handler services like League\Tactician\Bundle\Tests\Fake\FakeCommandHandler and then tell me if 2 column layout works for you :)

weaverryan · 2018-04-28T14:26:14Z

As we talked about, the table layout I suggested won’t work - class names are too long. So, the original format should be used, but probably with some added coloration to make it nice and clear :)

sroze

Looks good. Just two minor comments.

sroze · 2018-04-30T16:39:10Z

src/Symfony/Bundle/FrameworkBundle/Resources/config/console.xml

@@ -77,6 +77,11 @@
            <tag name="console.command" command="messenger:consume-messages" />
        </service>

+        <service id="console.command.messenger_debug" class="Symfony\Component\Messenger\Command\DebugMessengerCommand">


All the commands are named NameDebugCommand, not DebugNameCommand. Could you change it to be something consistent with the rest?

All the commands except DebugAutowiringCommand.. which i took as a template 😅 now updated.

sroze · 2018-04-30T16:39:48Z

src/Symfony/Component/Messenger/Command/DebugMessengerCommand.php

+    protected function configure()
+    {
+        $this
+            ->setDescription('Lists classes you can dispatch using the message bus')


I suggest "message classes" or "messages" (as you say "The following messages [...]" in the output)

chalasr

Looks good. A functional test case would have been nice, maybe could you just share a screenshot of the final output?

weaverryan · 2018-05-03T13:07:14Z

Here's a current screenshot, which I think looks quite good :).

We could perhaps do it in a different PR, but what about showing where each message is routed?

weaverryan · 2018-05-03T13:08:54Z

src/Symfony/Component/Messenger/Command/MessengerDebugCommand.php

+                $tableRows[] = array(sprintf('    handled by %s', $handler));
+            }
+        }
+        $io->table(array(), $tableRows);


Minor, but we should print some message here if there are NO messages, like:

No messages were found that have valid handlers.

@ro0NL can you update this PR at some point? Will be for beta2 now but can still be in 4.1 :)

Changes have been made 20+ days ago :)

yceruto

👍

yceruto · 2018-05-08T16:31:26Z

src/Symfony/Component/Messenger/Command/MessengerDebugCommand.php

+ *
+ * @experimental in 4.1
+ */
+class MessengerDebugCommand extends Command


Shouldn't it be named DebugCommand like debug commands in other components?

Good point @yceruto, you are right. Commands within components' namespace are not prefixed with the component name, let's keep it this way :)

Renamed 👍

Change have been made

ro0NL · 2018-05-09T09:09:16Z

Thx @sroze .. still no clue about failing test :(

ogizanagi · 2018-05-09T10:04:09Z

@ro0NL : deps=high failure expected. For deps=low, try updating the Framework bundle composer.json file to require symfony/messenger ~4.1-beta2

ogizanagi · 2018-05-09T11:25:45Z

The -beta2 part is important, as prefer-stable and prefer-lowest is used, so it selects the released beta1 first when using just @beta

ro0NL · 2018-05-09T11:35:49Z

your're right. switched it to @dev. Not sure beta2 will resolve... :S should we try? 😓

ogizanagi · 2018-05-09T11:42:36Z

That's usually what is done, so unless I'm missing something it should work: e81005e. It should resolve as far as minimum-stability allows it (which does)

sroze · 2018-05-09T11:59:33Z

Yay, and tests are all green now. Thanks @ogizanagi for the hint.

sroze · 2018-05-09T14:41:03Z

Thanks @ro0NL for working on this feature, this is much appreciated.

…oze) This PR was merged into the 4.1 branch. Discussion ---------- [Messenger] Add debug:messenger CLI command | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no  | Deprecations? | no | Tests pass? | yes  | Fixed tickets | #...  | License | MIT | Doc PR | symfony/symfony-docs#...  Adds a `debug:messenger` CLI command to expose message classes that can be dispatched. Heavily inspired by `debug:autowiring`. Commits ------- 7f87309 fix deps 290c7eb Rename the command `DebugCommand` 9847b83 [Messenger] Add debug:messenger CLI command

carsonbot added Status: Needs Review Feature labels Apr 4, 2018

ogizanagi reviewed Apr 4, 2018

View reviewed changes

ogizanagi added the Messenger label Apr 4, 2018

nicolas-grekas added this to the 4.1 milestone Apr 5, 2018

carsonbot removed the Status: Needs Review label Apr 6, 2018

fabpot previously requested changes Apr 6, 2018

View reviewed changes

carsonbot added the Status: Needs Work label Apr 6, 2018

sroze suggested changes Apr 8, 2018

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels Apr 30, 2018

sroze approved these changes Apr 30, 2018

View reviewed changes

chalasr approved these changes May 3, 2018

View reviewed changes

weaverryan previously requested changes May 3, 2018

View reviewed changes

carsonbot added Status: Needs Work and removed Status: Reviewed labels May 3, 2018

nicolas-grekas changed the base branch from master to 4.1 May 7, 2018 15:06

yceruto reviewed May 8, 2018

View reviewed changes

ro0NL and others added 2 commits May 9, 2018 09:27

[Messenger] Add debug:messenger CLI command

9847b83

Rename the command DebugCommand

290c7eb

carsonbot added Status: Reviewed and removed Status: Needs Work labels May 9, 2018

sroze approved these changes May 9, 2018

View reviewed changes

fix deps

7f87309

sroze merged commit 7f87309 into symfony:4.1 May 9, 2018

ro0NL deleted the debug-messenger branch May 10, 2018 15:16

fabpot mentioned this pull request May 21, 2018

Release v4.1.0-BETA2 #27331

Merged

ogizanagi mentioned this pull request Oct 12, 2018

[Messenger] Debug CLI command symfony/symfony-docs#10478

Closed

[Messenger] Add debug:messenger CLI command #26803

[Messenger] Add debug:messenger CLI command #26803

Conversation

ro0NL commented Apr 4, 2018

ogizanagi commented Apr 4, 2018

ogizanagi Apr 4, 2018 • edited Loading

Choose a reason for hiding this comment

ro0NL commented Apr 4, 2018 • edited Loading

ogizanagi commented Apr 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL commented Apr 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weaverryan commented Apr 21, 2018

sroze commented Apr 25, 2018

ro0NL commented Apr 28, 2018

weaverryan commented Apr 28, 2018

sroze left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chalasr left a comment

Choose a reason for hiding this comment

weaverryan commented May 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yceruto left a comment

Choose a reason for hiding this comment

yceruto May 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ro0NL commented May 9, 2018

ogizanagi commented May 9, 2018 • edited Loading

ogizanagi commented May 9, 2018 • edited Loading

ro0NL commented May 9, 2018

ogizanagi commented May 9, 2018

sroze commented May 9, 2018

sroze commented May 9, 2018

ogizanagi Apr 4, 2018 •

edited

Loading

ro0NL commented Apr 4, 2018 •

edited

Loading

yceruto May 8, 2018 •

edited

Loading

ogizanagi commented May 9, 2018 •

edited

Loading

ogizanagi commented May 9, 2018 •

edited

Loading