Skip to content
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

Create 'quorum' queues in RabbitMQ #3699

Merged
merged 8 commits into from
Sep 25, 2023
Merged

Conversation

aaguilartablada
Copy link
Contributor

It closes issue #3685

@aaguilartablada aaguilartablada requested review from a team as code owners August 28, 2023 06:25
Signed-off-by: Álvaro Aguilar <[email protected]>
Copy link
Collaborator

@hhunter-ms hhunter-ms left a comment

Choose a reason for hiding this comment

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

small edit

…ubsub/setup-rabbitmq.md

Co-authored-by: Hannah Hunter <[email protected]>
Signed-off-by: Álvaro Aguilar-Tablada Espinosa <[email protected]>
@aaguilartablada
Copy link
Contributor Author

changes applied. thanks @hhunter-ms !

@github-actions
Copy link

github-actions bot commented Sep 7, 2023

Stale PR, paging all reviewers

@msfussell
Copy link
Member

msfussell commented Sep 12, 2023

@aaguilartablada - The queuetype property needs to added to the Component format Metadata. The section you added is a Wow to example, but we need the overall reference in the table. Actually I now see this old PR. #3193. Can you pull the information from #3193 into this PR?

@aaguilartablada
Copy link
Contributor Author

@aaguilartablada - The queuetype property needs to added to the Component format Metadata. The section you added is a Wow to example, but we need the overall reference in the table. Actually I now see this old PR. #3193. Can you pull the information from #3193 into this PR?

The PR #3193 is bad documentation, as my PR dapr/components-contrib#2816 only allows classic and quorum queues. It does not include support for stream.

I think my documentation is OK as queue type is indicated at subscription level, not at component level.

Signed-off-by: Hannah Hunter <[email protected]>
@hhunter-ms
Copy link
Collaborator

resolved conflicts

@msfussell
Copy link
Member

@aaguilartablada - Look at my comment here https://github.com/dapr/docs/pull/3193/files#r1326422581
We do need to include all the metadata options and you have added queuetypein this PR. So can we merge #3193 along with this PR? It is unfortunate there are two docs PRs for this one code update.

@aaguilartablada
Copy link
Contributor Author

@msfussell let me insist, please.

My PR dapr/components-contrib#2816 does not include metadata for RabbitMQ pubsub component. It includes metadata for subscription the same way subscription contains metadata for routing key without including it in component. The comment you just mentioned is to document a pull request that was closed without merging. You can go to the issue associated dapr/components-contrib#2544 and then you will see that the PR dapr/components-contrib#2556 is closed. This PR tried to add metadata to pubsub component, right. But my solution, that was already merged, has another approach: configuring the queue type through subscription.

aaguilartablada and others added 2 commits September 18, 2023 08:47
…ubsub/setup-rabbitmq.md

Co-authored-by: Mark Fussell <[email protected]>
Signed-off-by: Álvaro Aguilar-Tablada Espinosa <[email protected]>
@github-actions
Copy link

Stale PR, paging all reviewers

@github-actions github-actions bot added the stale label Sep 24, 2023
@msfussell msfussell merged commit ad50b79 into dapr:v1.12 Sep 25, 2023
4 checks passed
@yaron2 yaron2 added this to the 1.12 milestone Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants