Skip to content

Update RabbitMQ.Client to version 7.0.0#8797

Merged
tobias-tengler merged 8 commits into
ChilliCream:mainfrom
ittennull:update-rabbitmq
Oct 20, 2025
Merged

Update RabbitMQ.Client to version 7.0.0#8797
tobias-tengler merged 8 commits into
ChilliCream:mainfrom
ittennull:update-rabbitmq

Conversation

@ittennull
Copy link
Copy Markdown
Contributor

@ittennull ittennull commented Oct 12, 2025

Summary of the changes (Less than 80 chars)

  • Update RabbitMQ.Client to version 7.0.0
  • Refactor topic implementations to always use IAsyncDisposable

Closes #8381

Old version of RabbitMQ.Client creates runtime errors if a projects uses 7.* version of RabbitMQ.Client or any dependency that rely on it, for example Masstransit

Refactor topic implementations to always use IAsyncDisposable
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Oct 12, 2025

CLA assistant check
All committers have signed the CLA.

@tobias-tengler
Copy link
Copy Markdown
Member

Thank you for the contribution @ittennull :)

Some tests are still failing. It would be great if you could get them to pass.

@tobias-tengler tobias-tengler marked this pull request as draft October 12, 2025 08:39
@ittennull
Copy link
Copy Markdown
Contributor Author

I fixed one test but all the RabbitMQ tests are red because the test library Squadron.RabbitMQ also uses outdated RMQ client which is not compatible with the new version. They don't have an update for it yet.
I ran the tests with RMQ running on localhost (I replaced connection values in test) and everything was green. However, I understand that it's not sufficient.
Maybe we can look into NOT using Squadron for RMQ?

@tobias-tengler tobias-tengler marked this pull request as ready for review October 12, 2025 10:53
Copilot AI review requested due to automatic review settings October 12, 2025 10:53
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the RabbitMQ.Client package from version 6.4.0 to 7.1.2 and refactors the topic implementations to consistently use IAsyncDisposable instead of IDisposable for better asynchronous resource management.

  • Updates RabbitMQ.Client to version 7.1.2 for compatibility with projects using newer versions
  • Refactors all topic implementations to use IAsyncDisposable pattern throughout the codebase
  • Migrates RabbitMQ test infrastructure from Squadron to Testcontainers

Reviewed Changes

Copilot reviewed 21 out of 22 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Directory.Packages.props Updates RabbitMQ.Client to 7.1.2 and adds Testcontainers packages
DefaultTopic.cs Changes session handling to use IAsyncDisposable instead of IDisposable
DefaultPubSub.cs Adds IsDisposed property for disposal state tracking
RabbitMQTopic.cs Major refactor to use async RabbitMQ.Client 7.x APIs and IAsyncDisposable
RabbitMQConnection.cs Complete rewrite to use async connection APIs and proper resource management
PostgresTopic.cs Updates to return IAsyncDisposable from OnConnectAsync
NatsTopic.cs Updates Session class to implement IAsyncDisposable
RedisTopic.cs Removes synchronous Dispose method, keeping only async disposal
Test files Updates test infrastructure and fixes disposal patterns
Files not reviewed (1)
  • src/HotChocolate/Core/src/Subscriptions.RabbitMQ/Properties/RabbitMQResources.Designer.cs: Language not supported

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/HotChocolate/Core/src/Subscriptions.RabbitMQ/RabbitMQTopic.cs
Comment thread src/HotChocolate/Core/src/Subscriptions.RabbitMQ/RabbitMQTopic.cs
@tobias-tengler
Copy link
Copy Markdown
Member

Thanks for fixing up the tests!
I like the switch to Testcontainers, but I'll check with the team first to confirm there wasn't a specific reason we chose Squadron originally.

@ittennull
Copy link
Copy Markdown
Contributor Author

ittennull commented Oct 12, 2025

I'll check with the team first to confirm there wasn't a specific reason we chose Squadron originally.

TestContainers work better here because the package only starts and stops containers and it doesn't depend on RabbitMq.Client library at all, so it's safer for any future updates

@ittennull
Copy link
Copy Markdown
Contributor Author

I'll check with the team first to confirm there wasn't a specific reason we chose Squadron originally.

@tobias-tengler Hi, did you have a chance to confer with the team yet?

@tobias-tengler
Copy link
Copy Markdown
Member

@ittennull Yes, we're going to stick with Squadron for now.
We just released a new version that should be compatible: https://github.com/SwissLife-OSS/squadron/releases/tag/0.26.0
Could you please undo your TestContainer changes and update to that Squadron version instead?

Also, please downgrade RabbitMQ.Client to just 7.0.0. We do not want to force people to use a specific minor/patch version.

@ittennull
Copy link
Copy Markdown
Contributor Author

OK, will do. The update of Squadron failed though

@ittennull
Copy link
Copy Markdown
Contributor Author

I saw that you updated Squadron to support RMQ 7 but the build fails because of the build scripts. I created a PR to fix it but nobody reviews it. I even wrote an email to the person who created these build scripts - no response.
I'm not sure if Squadron is a good dependency, in 6 years there are 100 commits, no updates, the github insights shows that the project is practically dead, the authors don't review PRs. Perhaps for the sake of hotchocolate you guys can re-consider if moving to another testing library is beter

@tobias-tengler
Copy link
Copy Markdown
Member

tobias-tengler commented Oct 16, 2025

@ittennull Good catch with the .slnx. I'll just undo my change to the solution file in the Squadron repository. Let's hope this fixes the issue.
In the short term @michaelstaib wants to move Squadron to this repository, as it was primarily built as test tooling for Hot Chocolate. This would also get rid of dependency issues like this.
Sorry for the delay here!

@tobias-tengler
Copy link
Copy Markdown
Member

@ittennull Squadron v26.0.0 is now available :)

@ittennull ittennull changed the title Update RabbitMQ.Client to version 7.1.2 Update RabbitMQ.Client to version 7.0.0 Oct 17, 2025
@ittennull
Copy link
Copy Markdown
Contributor Author

@tobias-tengler I updated the package. Could you approve the workflow and check the PR again?

Copy link
Copy Markdown
Member

@tobias-tengler tobias-tengler left a comment

Choose a reason for hiding this comment

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

@ittennull Looks good! :) I just have couple of questions.

Comment thread src/HotChocolate/Core/src/Subscriptions.RabbitMQ/RabbitMQPubSub.cs
Comment thread src/HotChocolate/Core/src/Subscriptions.RabbitMQ/RabbitMQPubSub.cs
ittennull and others added 2 commits October 18, 2025 06:12
- pass rabbitMqSubscriptionOptions to AddRabbitMQSubscriptionPublisher
- add ability to use server-generated RMQ queue names
- make queue auto-deletable
@ittennull
Copy link
Copy Markdown
Contributor Author

I also marked the queue as auto-delete because I noticed that we have thousands of messages and queues lying around with zero consumers. That happens when a graphql subscription finishes and a consumer unsubscribes. A queue is exclusive and isn't deleted because the connection that created it still lives and serves other new queues. So the only way to clean RMQ memory and storage is to restart your app

Copy link
Copy Markdown
Member

@tobias-tengler tobias-tengler left a comment

Choose a reason for hiding this comment

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

@ittennull Thanks for addressing my questions and thanks again for your time / contribution :)

@tobias-tengler tobias-tengler merged commit 600a2d5 into ChilliCream:main Oct 20, 2025
428 of 435 checks passed
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 20, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (8cd58f7) to head (f8c1b5d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@     Coverage Diff      @@
##   main   #8797   +/-   ##
============================
============================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

Subscriptions.RabbitMQ Dependency Upgrade

5 participants