-
-
Notifications
You must be signed in to change notification settings - Fork 498
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
RabbitMQ 3.7.9+ list compatibility and provider cleanup #759
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.
This looks pretty clean to me, and while using the API or using json out where supported would be better, this seems like a good incremental fix. Going to flag a couple other folks for review since this is a pretty big change. Can you also squash, either down to one commit or into logical commits?
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.
This looks good to me, but I'm probably not the best person to review changes to types and providers.
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.
Fantastic contribution! Thanks!
Module has acceptance tests that still pass, so +1 from me.
end | ||
|
||
# Retry the given code block 'count' retries or until the | ||
# command suceeeds. Use 'step' delay between retries. |
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.
Spelling of ‘succeeds’
Thanks for the contribution @JayH5 |
Thanks!
Yeah that's fine. Would've done that myself but just pushed this PR as I was ending my work day. |
Great! We are trying to cut a release of this soon. |
* RabbitMQ 3.7.9+ list compatibility and provider cleanup (voxpupuli#759)
Pull Request (PR) description
This is an alternative attempt at fixing
rabbitmqctl list_{x}
commands with RabbitMQ 3.7.9+. #751 was also an attempt. This PR takes a different approach by adding a class method in the "parent" provider class for all the providers. This method provides a consistent way to runrabbitmqctl list_
operations in all provider classes.This PR also cleans up the process of finding RabbitMQ CLI binaries: all providers now share the same command methods. It also adds caching to the RabbitMQ version value.
This Pull Request (PR) fixes the following issues
Fixes #740