Skip to content

Using AMQP auto-setup in all cases, not just in debug #30579

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
Mar 17, 2019

Conversation

weaverryan
Copy link
Member

Q A
Branch? master
Bug fix? yes and no
New feature? no
BC breaks? yes
Deprecations? no->
Tests pass? yes
Fixed tickets Related to #29476
License MIT
Doc PR TODO

Currently AMQP does auto-setup of queues/exchanges in dev-mode only. That's a problem for 2 reasons:

  1. Behavior in prod is drastically different... and actually... there's not currently a way I know of (easily) to set things up on prod.

  2. One of the properties of AMQP is that you typically DO want things to be set up at runtime, as you need them - you usually do want auto-setup.

This changes the behavior to auto-setup true always.

@nicolas-grekas nicolas-grekas added this to the next milestone Mar 16, 2019
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

(with minor typo)
another step forward for Messenger :)

@weaverryan weaverryan force-pushed the amqp-always-auto-setup-default branch from 069ab5f to 503c209 Compare March 17, 2019 00:40
@weaverryan
Copy link
Member Author

Ready now!

@sroze
Copy link
Contributor

sroze commented Mar 17, 2019

Good point 👍

@fabpot
Copy link
Member

fabpot commented Mar 17, 2019

Thank you @weaverryan.

@fabpot fabpot merged commit 503c209 into symfony:master Mar 17, 2019
fabpot added a commit that referenced this pull request Mar 17, 2019
…(weaverryan)

This PR was merged into the 4.3-dev branch.

Discussion
----------

Using AMQP auto-setup in all cases, not just in debug

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes and no
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no->
| Tests pass?   | yes
| Fixed tickets | Related to #29476
| License       | MIT
| Doc PR        | TODO

Currently AMQP does auto-setup of queues/exchanges in dev-mode only. That's a problem for 2 reasons:

1) Behavior in prod is drastically different... and actually... there's not currently a way I know of (easily) to set things up on prod.

2) One of the properties of AMQP is that you typically DO want things to be set up at runtime, as you need them - you usually *do* want auto-setup.

This changes the behavior to auto-setup true always.

Commits
-------

503c209 Using AMQP auto-setup in all cases, not just in debug
@weaverryan weaverryan deleted the amqp-always-auto-setup-default branch March 17, 2019 14:08
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019
@fabpot fabpot mentioned this pull request May 9, 2019
@makasim
Copy link
Contributor

makasim commented Nov 28, 2019

It does setup on EVERY consumed message, is it?. It, for sure, decreases consumption performance. That might also put some undesired load on RabbitMQ itself for it does four actions instead of one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants