Skip to content

[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

Merged
merged 1 commit into from
Jan 1, 2021

Conversation

jderusse
Copy link
Member

@jderusse jderusse commented Dec 22, 2020

Q A
Branch? 5.x
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #39605, Fix #38092, Fix #32172
License MIT
Doc PR -

To ease the setup, this PR also merge setup of exchange AND delayExchange.

/cc @Nyholm

@jderusse jderusse force-pushed the messenger-setup-once branch from 24afac6 to 5285ae1 Compare December 22, 2020 12:12
@chalasr
Copy link
Member

chalasr commented Dec 22, 2020

Should also fix #38092 and maybe #32172.

@jderusse jderusse force-pushed the messenger-setup-once branch 2 times, most recently from 0db080e to f4054c1 Compare December 23, 2020 10:36
@nicolas-grekas nicolas-grekas added this to the 5.x milestone Dec 23, 2020
@jderusse jderusse force-pushed the messenger-setup-once branch from f4054c1 to c2e84c6 Compare December 23, 2020 19:47
Copy link
Member

@chalasr chalasr left a 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

@fabpot
Copy link
Member

fabpot commented Jan 1, 2021

Thank you @jderusse.

@fabpot fabpot merged commit 61aa8fd into symfony:5.x Jan 1, 2021
@jderusse jderusse deleted the messenger-setup-once branch January 2, 2021 22:22
@@ -452,8 +447,11 @@ public function nack(\AMQPEnvelope $message, string $queueName, int $flags = \AM

public function setup(): void
{
$this->setupExchangeAndQueues();
if ($this->autoSetupExchange) {
Copy link
Contributor

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in #40966

fabpot added a commit that referenced this pull request Apr 29, 2021
…(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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants