-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Messenger] Setup queues once in AMQP #39608
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
src/Symfony/Component/Messenger/Bridge/Amqp/Transport/Connection.php
Outdated
Show resolved
Hide resolved
24afac6
to
5285ae1
Compare
src/Symfony/Component/Messenger/Bridge/Amqp/Tests/Transport/ConnectionTest.php
Outdated
Show resolved
Hide resolved
src/Symfony/Component/Messenger/Bridge/Amqp/Tests/Transport/ConnectionTest.php
Outdated
Show resolved
Hide resolved
0db080e
to
f4054c1
Compare
src/Symfony/Component/Messenger/Bridge/Amqp/Transport/Connection.php
Outdated
Show resolved
Hide resolved
f4054c1
to
c2e84c6
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.
Looks good to me, Thanks
Thank you @jderusse. |
@@ -452,8 +447,11 @@ public function nack(\AMQPEnvelope $message, string $queueName, int $flags = \AM | |||
|
|||
public function setup(): void | |||
{ | |||
$this->setupExchangeAndQueues(); | |||
if ($this->autoSetupExchange) { |
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.
The setup seems broken now. The setup method is public because it's called by SetupableTransportInterface. Now if you disable auto_setup and want to setup the transports manually, then it does not actually setup the exchanges and queues anymore. But that is the whole point of the setup method: being able to setup manually when autosetup is not used.
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.
Fixed in #40966
…(Tobion) This PR was merged into the 5.3-dev branch. Discussion ---------- [Messenger] fix manual amqp setup when autosetup disabled | Q | A | ------------- | --- | Branch? | 5.x | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tickets | Fix #39608 (comment) | License | MIT | Doc PR | Commits ------- 29e5bde [Messenger] fix manual amqp setup when autosetup disabled
To ease the setup, this PR also merge setup of exchange AND delayExchange.
/cc @Nyholm