-
Notifications
You must be signed in to change notification settings - Fork 41.2k
Provide a property for configuring Spring AMQP's address shuffle mode #23091
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
Provide a property for configuring Spring AMQP's address shuffle mode #23091
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion and the PR. I've made a few comments, can you please have a look to those and also add the necessary tests to exercise the code that you've changed?
/** | ||
* Shuffling mode for connecting host. Default mode is NONE. | ||
*/ | ||
private String addressShuffleMode = "NONE"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The enum should be exposed rather as it provides better metadata
@@ -87,6 +89,11 @@ | |||
*/ | |||
private String addresses; | |||
|
|||
/** | |||
* Shuffling mode for connecting host. Default mode is NONE. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default value should not be documented as it's the role of the configuration metadata to do that (look at other keys for inspiration).
* @see #setAddressShuffleMode(String) | ||
* @see #getAddressShuffleMode() | ||
*/ | ||
public AbstractConnectionFactory.AddressShuffleMode determineAddressShuffleMode() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should do that. Rather we should set whatever has been set by the user.
@snicoll , i have edited what you mention. please re-review plz. |
@01045972746 thank you for making your first contribution to Spring Boot. I've polished your contribution and added a test. |
This support connecting to RabbitMQ clustered host addresses with shuffling.
https://docs.spring.io/spring-amqp/reference/html/#cluster
Because of
shuffle-addresses
mode is deprecated, i have addedaddressShuffleMode
property which isString
.Please review and comment. Thanks.