Add `symfony/messenger` recipe #395

sroze · 2018-03-27T14:46:44Z

Q	A
License	MIT

Depends on [Messenger] Add AMQP adapter symfony#26632.

ghost

Pull request does not pass validation.

ghost

Pull request does not pass validation.

sroze · 2018-03-27T14:49:25Z

I guess the review is invalid because obviously, the tag does not exist 🤔

Pierstoval · 2018-03-27T21:25:22Z

symfony/messenger/4.1/manifest.json

+    "env": {
+        "MESSENGER_ADAPTER_DSN": "amqp://guest:guest@localhost:5672/%2f/messages"
+    },
+    "aliases": ["messenger", "amqp"]


I'd also add "messaging" alias, WDYT?

What about command_bus too? Even if that's a bit reductive considering the wider component scope, that might be a good way to discover it.

I like it: when making presentations about flex & hexagonal architecture, it's always badass to say "We need a command bus, so let's do composer req command_bus 😄

I'm not sure about "messaging" tbh. command-bus might be better as a meta-package (or other package), which will use the Messenger and auto-enable specific middlewares for command buses (see here)

ogizanagi · 2018-03-28T11:11:59Z

symfony/messenger/4.1/manifest.json

+    "env": {
+        "MESSENGER_ADAPTER_DSN": "amqp://guest:guest@localhost:5672/%2f/messages"
+    },
+    "aliases": ["messenger", "amqp"]


What about command_bus too? Even if that's a bit reductive considering the wider component scope, that might be a good way to discover it.

ogizanagi · 2018-03-28T11:14:40Z

symfony/messenger/4.1/manifest.json

+        "config/": "%CONFIG_DIR%/"
+    },
+    "env": {
+        "MESSENGER_ADAPTER_DSN": "amqp://guest:guest@localhost:5672/%2f/messages"


Should we really provide this env var value by default? If there is no need for an adapter at first, can't we set it in the recipe to have null as default and add a message explaining how to tweak it in order to use the amqp adapter?

I.e:

# config/packages/messenger.yaml parameters: env(MESSENGER_ADAPTER_DSN): null

or commenting adapters.default: "%env(MESSENGER_ADAPTER_DSN)%"
or providing the MESSENGER_ADAPTER_DSN="amqp://guest:guest@localhost:5672/%2f/messages" var commented by default, as in

recipes/symfony/framework-bundle/3.3/manifest.json

Line 17 in bd24721

"#TRUSTED_PROXIES": "127.0.0.1,127.0.0.2",

.

I quite like the idea of commented lines for the adapters and environment variable as well. I've updated the PR accordingly 👍

ghost

Pull request does not pass validation.

ghost

Pull request passes validation.

ghost

Pull request passes validation.

Nyholm

I’m happy with this recipe. I usually ask for a link to the documentation but we can add that later.

Nyholm · 2018-04-12T18:28:15Z

symfony/messenger/4.1/config/packages/messenger.yaml

+            # amqp: "%env(MESSENGER_ADAPTER_DSN)%"
+
+        routing:
+            # Route your messages to the senders...


Why the mysterious dots at the end ...?

No reason 😄

weaverryan · 2018-04-12T18:41:36Z

symfony/messenger/4.1/config/packages/messenger.yaml

+
+        routing:
+            # Route your messages to the senders...
+            # 'App\Message\YourMessage': messenger.sender.amqp


We're discussing allowing the short adapter name here - e.g. amqp (and also service id's, of course). If we do that (I want to), it might be simpler to do that before merging the recipe.

Fair point. For the records, it's here: symfony/symfony#26909

ghost

Pull request passes validation.

ghost

Pull request passes validation.

fabpot · 2018-04-17T06:12:10Z

symfony/messenger/4.1/config/packages/messenger.yaml

+            # amqp: "%env(MESSENGER_ADAPTER_DSN)%"
+
+        routing:
+            # Route your messages to the senders.


I would even remove this last dot.

Here we go, removed (and actually replaced "senders" with "adapters" as it makes more sense now we the shorter name I believe).

ghost

Pull request passes validation.

yceruto · 2018-04-27T22:51:59Z

symfony/messenger/4.1/config/packages/messenger.yaml

+    messenger:
+        adapters:
+            # Uncomment the following line to enable an adapter named "amqp"
+            # amqp: "%env(MESSENGER_ADAPTER_DSN)%"


Single quote around the parameter?

ogizanagi · 2018-05-08T20:16:56Z

symfony/messenger/4.1/manifest.json

+    },
+    "env": {
+        "#MESSENGER_ADAPTER_DSN": "amqp://guest:guest@localhost:5672/%2f/messages"
+    },


Perhaps post-install-output would be a good fit for mentioning the symfony/serializer-pack to start using built-in transport easily.

Maybe a new symfony/messenger-pack would help to install both also.

Any wording suggestion? :)

<bg=blue;fg=white> </> <bg=blue;fg=white> What's next? </> <bg=blue;fg=white> </> * You're ready to use the Messenger component. You can define your own message buses or start using the default one right now by injecting the <info>messenger.bus.default</> service or typehinting <info>Symfony\Component\Messenger\MessageBusInterface</> in your code. * If you need to send messages to your broker, you can benefit from the built-in amqp transport by: 1. Installing the <info>symfony/serializer-pack</> 2. Uncommenting the <comment> MESSENGER_ADAPTER_DSN</> env var and `framework.messanger.transports.amqp` config 3. Routing your messages to the amqp sender. * <fg=blue>Read</> the documentation at <comment>https://symfony.com/doc/master/components/messenger.html</>

(something like that)

That's an amazing idea! Added a very similar content 👍

sroze · 2018-05-08T20:40:42Z

symfony/messenger/4.1/config/packages/messenger.yaml

@@ -0,0 +1,9 @@
+framework:
+    messenger:
+        adapters:


Should be renamed to transports

ghost

Pull request passes validation.

ghost

Pull request passes validation.

fabpot · 2018-05-11T12:00:02Z

symfony/messenger/4.1/post-install.txt

+    or typehinting <info>Symfony\Component\Messenger\MessageBusInterface</info> in your code.
+
+  * If you need to send messages to your broker, you can benefit from the built-in
+    amqp transport by:


fabpot · 2018-05-11T12:00:19Z

symfony/messenger/4.1/post-install.txt

+  * If you need to send messages to your broker, you can benefit from the built-in
+    amqp transport by:
+
+    1. Installing the <info>symfony/serializer-pack</>


missing ; at the end (same for the next bullet)

fabpot · 2018-05-11T12:00:52Z

symfony/messenger/4.1/post-install.txt

+
+    1. Installing the <info>symfony/serializer-pack</>
+    2. Uncommenting the <comment> MESSENGER_ADAPTER_DSN</> env var
+       and `framework.messanger.transports.amqp` config


You need to remove the backticks. Replace them with <comment>?

fabpot · 2018-05-11T12:00:58Z

symfony/messenger/4.1/post-install.txt

+    amqp transport by:
+
+    1. Installing the <info>symfony/serializer-pack</>
+    2. Uncommenting the <comment> MESSENGER_ADAPTER_DSN</> env var


extra whitespace after <comment>

ghost

Pull request passes validation.

sroze · 2018-05-11T12:31:00Z

@fabpot updated and added the amqp alias on the symfony/amqp-pack.

fabpot · 2018-05-11T13:33:08Z

symfony/messenger/4.1/manifest.json

+        "config/": "%CONFIG_DIR%/"
+    },
+    "env": {
+        "#MESSENGER_ADAPTER_DSN": "amqp://guest:guest@localhost:5672/%2f/messages"


The # looks weird here. Keep in mind that this is going to be used for more than the .env file.

I would do something like: "#1": "MESSENGER_ADAPTER_DSN amqp://guest:guest@localhost:5672/%2f/messages", instead.

Well, that's what we do in the framework-bundle as well.

I'm not sure what you mean by #1 as a key 🤔

We use the same here:

recipes/symfony/framework-bundle/3.3/manifest.json

Line 17 in bd24721

"#TRUSTED_PROXIES": "127.0.0.1,127.0.0.2",

should we change it? Not sure to understand what you mean by

Keep in mind that this is going to be used for more than the .env file.

?

We are using the env config to update phpunit.xml as well. So, using # to mean "comment" is wrong. #1 is a convention to tell the parser that this is a comment, which is then interpreted depending on where we are using the env var.

#1 is a convention to tell the parser that this is a comment, which is then interpreted depending on where we are using the env var.

Alright, I'm going to change it that way; though... is it documented somewhere? 🤔

ghost

Pull request passes validation.

Nyholm

Excellent work. Thank you

Nyholm · 2018-05-16T09:21:49Z

@symfony-flex-server review please

fabpot · 2018-05-16T16:00:45Z

@symfony-flex-server review

sroze · 2018-05-16T16:14:09Z

That bot isn't really listening. 😛

ghost

Pull request passes validation.

Add messenger's recipe

388a5c4

ghost suggested changes Mar 27, 2018

View reviewed changes

sroze force-pushed the add-messenger-recipe branch from 43207f5 to 388a5c4 Compare March 27, 2018 14:47

ghost suggested changes Mar 27, 2018

View reviewed changes

Pierstoval reviewed Mar 27, 2018

View reviewed changes

ogizanagi reviewed Mar 28, 2018

View reviewed changes

ogizanagi mentioned this pull request Mar 29, 2018

[Messenger] Add a MessageHandlerInterface (multiple messages + auto-configuration) symfony/symfony#26685

Merged

Comment the adapter and env as well

59dd763

ghost suggested changes Mar 29, 2018

View reviewed changes

Uses amqp as the default sender name

5151546

ghost deployed to staging April 12, 2018 18:23 View deployment

ghost approved these changes Apr 12, 2018

View reviewed changes

Change wording in comment

3fc2685

ghost deployed to staging April 12, 2018 18:24 View deployment

ghost approved these changes Apr 12, 2018

View reviewed changes

Nyholm reviewed Apr 12, 2018

View reviewed changes

weaverryan reviewed Apr 12, 2018

View reviewed changes

Uses the short sender name and remove mysterious dots

ef00c5a

ghost deployed to staging April 12, 2018 19:32 View deployment

ghost approved these changes Apr 12, 2018

View reviewed changes

ghost deployed to staging April 12, 2018 19:32 View deployment

ghost approved these changes Apr 12, 2018

View reviewed changes

fabpot requested changes Apr 17, 2018

View reviewed changes

Remove last dot and change sender for adapter

5b86e34

ghost deployed to staging April 17, 2018 06:46 View deployment

ghost approved these changes Apr 17, 2018

View reviewed changes

yceruto reviewed Apr 27, 2018

View reviewed changes

ogizanagi reviewed May 8, 2018

View reviewed changes

sroze commented May 8, 2018

View reviewed changes

sroze added 2 commits May 9, 2018 19:44

Add a post-install note

dfda2d4

Add the AMQP pack

58ae22c

ghost deployed to staging May 9, 2018 18:44 View deployment

ghost approved these changes May 9, 2018

View reviewed changes

sroze force-pushed the add-messenger-recipe branch from 0cfacbe to dfda2d4 Compare May 9, 2018 18:45

ghost deployed to staging May 9, 2018 18:45 View deployment

ghost approved these changes May 9, 2018

View reviewed changes

Installing

e0988a5

fabpot reviewed May 11, 2018

View reviewed changes

fabpot requested changes May 11, 2018

View reviewed changes

sroze added 2 commits May 11, 2018 14:24

Update some typos

81580a1

Rename to amqp-pack

f3ed9ed

ghost deployed to staging May 11, 2018 12:30 View deployment

ghost approved these changes May 11, 2018

View reviewed changes

fabpot requested changes May 11, 2018

View reviewed changes

Uses @fabpot's suggestion for the environment

a2ead7b

ghost deployed to staging May 12, 2018 09:06 View deployment

ghost approved these changes May 12, 2018

View reviewed changes

fabpot approved these changes May 12, 2018

View reviewed changes

Nyholm approved these changes May 16, 2018

View reviewed changes

ghost deployed to staging May 16, 2018 19:01 View deployment

ghost approved these changes May 16, 2018

View reviewed changes

ghost merged commit a2ead7b into symfony:master May 16, 2018

ghost pushed a commit that referenced this pull request May 16, 2018

Merge pull request #395

34fc421

sroze deleted the add-messenger-recipe branch May 16, 2018 19:07

This pull request was closed.

Add symfony/messenger recipe #395

Add symfony/messenger recipe #395

Conversation

sroze commented Mar 27, 2018 • edited

ghost left a comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

sroze commented Mar 27, 2018 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

Nyholm 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

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ogizanagi May 8, 2018 • edited

Choose a reason for hiding this comment

yceruto May 8, 2018 • edited

Choose a reason for hiding this comment

sroze May 8, 2018 • edited

Choose a reason for hiding this comment

ogizanagi May 8, 2018 • edited

Choose a reason for hiding this comment

sroze May 9, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

ghost 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

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

sroze commented May 11, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

Nyholm left a comment

Choose a reason for hiding this comment

Nyholm commented May 16, 2018

fabpot commented May 16, 2018

sroze commented May 16, 2018

ghost left a comment

Choose a reason for hiding this comment

Add `symfony/messenger` recipe #395

Add `symfony/messenger` recipe #395

sroze commented Mar 27, 2018 •

edited

sroze commented Mar 27, 2018 •

edited

ogizanagi May 8, 2018 •

edited

yceruto May 8, 2018 •

edited

sroze May 8, 2018 •

edited

ogizanagi May 8, 2018 •

edited

sroze May 9, 2018 •

edited