[Messenger] Wire the transaction middleware factory when component is enabled #817

ogizanagi · 2018-05-11T12:23:36Z

How should we process for the CI?

sroze · 2018-05-11T12:34:08Z

composer.json

@@ -61,5 +62,6 @@
        "branch-alias": {
            "dev-master": "1.9.x-dev"
        }
-    }
+    },
+    "minimum-stability": "dev"


Should that really be added?

That would be required to get the beta2 until released, but I don't think it'll stay here. That was just the way to install it locally for me.
Anyway, I'm waiting some instructions/plans/help to deal with the CI as we're even unlikely to keep the messenger component in require-dev due to the sf 2.7 support in this bundle.

yceruto · 2018-05-11T13:29:32Z

DependencyInjection/Compiler/MessengerPass.php

+    public function process(ContainerBuilder $container)
+    {
+        // Remove wired services if the Messenger component actually isn't enabled:
+        if (!$container->hasAlias('message_bus') || !is_subclass_of($container->findDefinition('message_bus')->getClass(), MessageBusInterface::class, true)) {


The 3rd argument of is_subclass_of() is already true by default, right?

true :)

PHP API consistency (see is_a) tricked me!

… enabled

ogizanagi · 2018-05-14T10:57:50Z

~~Should be ready. New tests on the new travis build should not be skipped anymore once symfony/symfony#27128 is merged.~~

Majkl578 · 2018-05-14T11:46:38Z

Tests/DependencyInjection/DoctrineExtensionTest.php

@@ -676,6 +677,31 @@ public function testAnnotationsBundleMappingDetectionWithVendorNamespace()
        $this->assertEquals('Fixtures\Bundles\Vendor\AnnotationsBundle\Entity', $calls[0][1][1]);
    }

+    public function testMessengerIntegration()
+    {
+        if (! interface_exists(MessageBusInterface::class)) {


I'd argue that it should be a dev-dependency, otherwise it's untested.

Oops, I though symfony/symfony was used on CI for specific versions. So I expected it to be available in the new build.
The issue with adding it into the dev-dependencies (see first commit) is that it'll conflict with lowest dependencies tested for this packages due to the component requirements. So it must be explicitly added on a specific build. WDYT?

You can override it for now using ^4.1@beta, the @beta part would be dropped once 4.1 is stable. But yes, it wouldn't allow SF pre-3.4. Since there is only one (or two) CI job to test SF 2.x, I think it'd be better to drop this dependency only in such job.
(Another argument for dropping 2.x support soon.)

That would actually require dropping symfony/messenger for every build without PHP 7.1, a minimum-stability: dev or with a specific SYMFONY_VERSION except for the new 4.1-beta2 one, as the component is only available since 4.1 and the required classes would be part of the beta2 (so it'll be quite a mess).
For now, I've just added the symfony/messenger component on the new build, so this build will pass once the related PR on symfony/symfony is merged.
But if there is a better strategy to you, please feel free to push the required changes to my branch :) Thank you!

sroze · 2018-05-14T19:22:27Z

PR has been merged on Symfony 👍

ogizanagi · 2018-05-14T19:30:28Z

And it's now green. Also tested on a real app, works perfectly fine :)

weaverryan · 2018-05-15T03:35:57Z

Can you open a docs PR on Symfony too?

ogizanagi · 2018-05-15T08:12:11Z

@weaverryan : Done in symfony/symfony-docs#9774

…iereguiluz) This PR was merged into the 4.1 branch. Discussion ---------- [Messenger] Document middleware factories  Refs: - Symfony PR: symfony/symfony#27128 - DoctrineBundle PR: doctrine/DoctrineBundle#817 Commits ------- c5ef7c8 Reowrd to restore the original meaning a87f21b Minor rewords 2880027 [Messenger] Document middleware factories

ogizanagi · 2018-05-30T16:25:38Z

Symfony 4.1 is released. Could we move forward with this one now? :)

(Note: Code styles issues are not related to these changes)

ogizanagi · 2018-07-05T07:07:47Z

Hi there 👋 Any update?
Symfony 4.1 is released since a month now, it would be awesome for end-users to be able to use the transaction middleware out-of-the-box thanks to this :)

sroze · 2018-07-26T21:03:52Z

Anybody? :)

Nyholm · 2018-08-26T16:05:09Z

Friendly ping

Nyholm

@ogizanagi Could you fix the CS issues (see Travis)

Ping @kimhemsoe could you have a look on this PR?

ogizanagi · 2018-09-09T19:16:43Z

@Nyholm : As mentioned previously, these CS issues aren't actually related to this PR :)

kimhemsoe · 2018-09-10T11:04:58Z

@ogizanagi Thanks

sroze · 2018-09-13T00:20:21Z

🎉

…

On Wed, 12 Sep 2018 at 21:54, Kim Hemsø Rasmussen ***@***.***> wrote: Merged #817 <#817> into master. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#817 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAxHEaaFcNN4pBjnmbsDccvndanceK9Xks5uaWZfgaJpZM4T7YED> .

ogizanagi mentioned this pull request May 11, 2018

[Messenger] Middleware factories support in config symfony/symfony#27128

Merged

sroze approved these changes May 11, 2018

View reviewed changes

yceruto reviewed May 11, 2018

View reviewed changes

ogizanagi force-pushed the messenger/transaction_middleware_factory branch from 6a7bbc6 to cf4d445 Compare May 11, 2018 15:33

[Messenger] Wire the transaction middleware factory when component is…

b04cc9e

… enabled

ogizanagi force-pushed the messenger/transaction_middleware_factory branch 2 times, most recently from 1fb8719 to 4fa24cd Compare May 14, 2018 10:40

Majkl578 reviewed May 14, 2018

View reviewed changes

ogizanagi force-pushed the messenger/transaction_middleware_factory branch 3 times, most recently from ca8c487 to 1436d7a Compare May 14, 2018 14:20

ogizanagi force-pushed the messenger/transaction_middleware_factory branch from 1436d7a to 0a39dc9 Compare May 14, 2018 19:25

ogizanagi mentioned this pull request May 15, 2018

[Messenger] Document middleware factories symfony/symfony-docs#9774

Merged

Update for CI & fix CS

f9ba543

ogizanagi force-pushed the messenger/transaction_middleware_factory branch from 0a39dc9 to f9ba543 Compare May 30, 2018 16:09

ogizanagi mentioned this pull request Jul 4, 2018

[Messenger] Show how to configure a doctrine transaction middleware symfony/symfony-docs#10013

Closed

Nyholm approved these changes Sep 9, 2018

View reviewed changes

kimhemsoe merged commit 140fb6d into doctrine:master Sep 12, 2018

ogizanagi deleted the messenger/transaction_middleware_factory branch September 12, 2018 20:31

kimhemsoe added this to the 1.9.2 milestone Oct 30, 2018

ostrolucky mentioned this pull request Nov 30, 2022

Drop support for Symfony 4 #1586

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Messenger] Wire the transaction middleware factory when component is enabled #817

[Messenger] Wire the transaction middleware factory when component is enabled #817

ogizanagi commented May 11, 2018 •

edited

sroze May 11, 2018

ogizanagi May 11, 2018 •

edited

yceruto May 11, 2018

ogizanagi May 11, 2018

ogizanagi commented May 14, 2018 •

edited

Majkl578 May 14, 2018

ogizanagi May 14, 2018

Majkl578 May 14, 2018 •

edited

ogizanagi May 14, 2018

sroze commented May 14, 2018

ogizanagi commented May 14, 2018

weaverryan commented May 15, 2018

ogizanagi commented May 15, 2018

ogizanagi commented May 30, 2018

ogizanagi commented Jul 5, 2018

sroze commented Jul 26, 2018

Nyholm commented Aug 26, 2018

Nyholm left a comment

ogizanagi commented Sep 9, 2018

kimhemsoe commented Sep 10, 2018

sroze commented Sep 13, 2018 via email

[Messenger] Wire the transaction middleware factory when component is enabled #817

[Messenger] Wire the transaction middleware factory when component is enabled #817

Conversation

ogizanagi commented May 11, 2018 • edited

sroze May 11, 2018

Choose a reason for hiding this comment

ogizanagi May 11, 2018 • edited

Choose a reason for hiding this comment

yceruto May 11, 2018

Choose a reason for hiding this comment

ogizanagi May 11, 2018

Choose a reason for hiding this comment

ogizanagi commented May 14, 2018 • edited

Majkl578 May 14, 2018

Choose a reason for hiding this comment

ogizanagi May 14, 2018

Choose a reason for hiding this comment

Majkl578 May 14, 2018 • edited

Choose a reason for hiding this comment

ogizanagi May 14, 2018

Choose a reason for hiding this comment

sroze commented May 14, 2018

ogizanagi commented May 14, 2018

weaverryan commented May 15, 2018

ogizanagi commented May 15, 2018

ogizanagi commented May 30, 2018

ogizanagi commented Jul 5, 2018

sroze commented Jul 26, 2018

Nyholm commented Aug 26, 2018

Nyholm left a comment

Choose a reason for hiding this comment

ogizanagi commented Sep 9, 2018

kimhemsoe commented Sep 10, 2018

sroze commented Sep 13, 2018 via email

ogizanagi commented May 11, 2018 •

edited

ogizanagi May 11, 2018 •

edited

ogizanagi commented May 14, 2018 •

edited

Majkl578 May 14, 2018 •

edited