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

Update RabbitMQ client version to 6.2.2 #174

Merged
merged 1 commit into from
Jan 14, 2022

Conversation

JatinSanghvi
Copy link
Contributor

@JatinSanghvi JatinSanghvi commented Oct 20, 2021

  • Updated RabbitMQ client version from 5.1.2 to 6.2.2.
  • Updated the target framework for sample and test projects. '.NET Core 2.2' is out of support since 2019.
  • Updated the code to work with a breaking change introduced in 6.2.2 client.
  • Fixed unit test to work with new client version.
  • Verified all sample functions work as expected with the changes.

Fixes #173, #92

kshyju
kshyju previously requested changes Oct 25, 2021
src/WebJobs.Extensions.RabbitMQ.csproj Show resolved Hide resolved
@JatinSanghvi JatinSanghvi marked this pull request as draft October 27, 2021 04:52
@JatinSanghvi JatinSanghvi changed the title Update RabbitMQ client version to 6.2.2 Draft: Update RabbitMQ client version to 6.2.2 Oct 27, 2021
@JatinSanghvi JatinSanghvi force-pushed the JatinSanghvi/update-rabbitmq-client branch 4 times, most recently from 85172c9 to abf30a3 Compare November 26, 2021 12:57
@JatinSanghvi JatinSanghvi marked this pull request as ready for review November 26, 2021 12:57
@JatinSanghvi JatinSanghvi requested a review from kshyju November 26, 2021 12:58
@JatinSanghvi JatinSanghvi force-pushed the JatinSanghvi/update-rabbitmq-client branch from abf30a3 to de18bfb Compare November 26, 2021 14:21
@JatinSanghvi JatinSanghvi changed the title Draft: Update RabbitMQ client version to 6.2.2 Update RabbitMQ client version to 6.2.2 Nov 26, 2021
@kshyju kshyju requested a review from yojagad November 29, 2021 20:25
@@ -26,7 +27,7 @@ public interface IRabbitMQModel

void BasicReject(ulong deliveryTag, bool requeue);

void BasicPublish(string exchange, string routingKey, IBasicProperties basicProperties, byte[] body);
void BasicPublish(string exchange, string routingKey, IBasicProperties basicProperties, ReadOnlyMemory<byte> body);
Copy link
Member

Choose a reason for hiding this comment

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

This interface is public. So, changing the method signature will be a breaking change for anyone using this API. Do you know who uses this?

Do we need this change? If so, should we do it in a backward compatible way? (Introduce a new overload and mark the old one as obsolete).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no public class that accepts IRabbitMQModel as its method argument or has IRabbitMQModel as return type. The only path where I could find the model definition leaking was via IRabbitMQModel <- IRabbitMQService <-IRabbitMQServiceFactory but it stops at that point. There are no public classes taking or returning either IRabbitMQService or IRabbitMQServiceFactory or any of their implementation classes. I guess customer would not be impacted if they cannot work with IRabbitMQModel in connection with any other classes in RabbitMQ extension. I will fix the visibility of the classes and interfaces in a future PR.

Copy link
Member

@kshyju kshyju left a comment

Choose a reason for hiding this comment

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

One question about the public API surface.

@yojagad
Copy link
Contributor

yojagad commented Nov 29, 2021

The general guidance is to introduce newer rabbitmq client version updates (major version updates) through a new major version of the extension.

@JatinSanghvi JatinSanghvi force-pushed the JatinSanghvi/update-rabbitmq-client branch from de18bfb to 34ba280 Compare January 9, 2022 12:02
@JatinSanghvi JatinSanghvi requested a review from kshyju January 9, 2022 14:08
@JatinSanghvi
Copy link
Contributor Author

JatinSanghvi commented Jan 10, 2022

The general guidance is to introduce newer rabbitmq client version updates (major version updates) through a new major version of the extension.

Following the same tradition as of other Azure Functions repo, I will fork out v1.x branch but dev branch will continue to be the one for next minor/major release which will be 2.0.0. So the PR has correct branch set as the target.

@JatinSanghvi JatinSanghvi requested review from lpapudippu and kshyju and removed request for kshyju and yojagad January 14, 2022 08:13
@JatinSanghvi JatinSanghvi dismissed kshyju’s stale review January 14, 2022 08:22

A manual version bump is not required with new build and release pipeline.

@JatinSanghvi JatinSanghvi merged commit 0441c88 into dev Jan 14, 2022
@JatinSanghvi JatinSanghvi deleted the JatinSanghvi/update-rabbitmq-client branch January 14, 2022 08:23
@JatinSanghvi JatinSanghvi linked an issue Jan 29, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants