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

RabbitMQ 3.7.9+ list compatibility and provider cleanup #759

Merged
merged 8 commits into from
Jan 15, 2019

Conversation

JayH5
Copy link
Contributor

@JayH5 JayH5 commented Jan 15, 2019

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 run rabbitmqctl 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

Copy link
Contributor

@wyardley wyardley left a 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?

Copy link
Member

@bastelfreak bastelfreak left a 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.

Copy link
Member

@alexjfisher alexjfisher left a 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.
Copy link
Member

Choose a reason for hiding this comment

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

Spelling of ‘succeeds’

@wyardley wyardley merged commit 42d45ed into voxpupuli:master Jan 15, 2019
@wyardley
Copy link
Contributor

Thanks for the contribution @JayH5
I know a ton of people will appreciate this
I squash-merged, since the commits weren't signed anyway; hope that's Ok.

@JayH5 JayH5 deleted the rabbitmq-3.7.9-list branch January 16, 2019 05:48
@JayH5
Copy link
Contributor Author

JayH5 commented Jan 16, 2019

Thanks!

I squash-merged, since the commits weren't signed anyway; hope that's Ok.

Yeah that's fine. Would've done that myself but just pushed this PR as I was ending my work day.

@wyardley
Copy link
Contributor

Great! We are trying to cut a release of this soon.

@wyardley wyardley added the bug Something isn't working label Jan 16, 2019
cegeka-jenkins pushed a commit to cegeka/puppet-rabbitmq that referenced this pull request Mar 26, 2021
* RabbitMQ 3.7.9+ list compatibility and provider cleanup (voxpupuli#759)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rabbitmqctl 'broken' in 3.7.9
4 participants