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

Extend version regex for RabbitMQ 3.8 #814

Merged
merged 1 commit into from
Oct 12, 2019

Conversation

codeinthehole
Copy link
Contributor

@codeinthehole codeinthehole commented Oct 10, 2019

Where the output from rabbitmqctl -q status isn't printed as an Erlang
property list.

Without this change, you get a crash with error:

Failed to apply catalog: undefined method `scan' for nil:NilClass

@codeinthehole codeinthehole changed the title #### Pull Request (PR) description Extend version regex for RabbitMQ 3.8 Extend version regex for RabbitMQ 3.8 Oct 10, 2019
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.

Hi -- I think you'll need to slightly adjust the stubbed commands in the tests - see the failed items in the Travis build.

Because, in RabbitMQ 3.8.0, the default output of `rabbitmqctl -q
status` has changed; it no longer formats output using Erlang's property
list format.

When making a version check, we also verify the version has been
extracted ok.
@codeinthehole
Copy link
Contributor Author

@wyardley Good point. Have fixed the tests and force-pushed back up.

@bastelfreak
Copy link
Member

@codeinthehole thanks for the PR. Can you add an acceptance test that verifies this works?

@bastelfreak bastelfreak added enhancement New feature or request needs-tests labels Oct 12, 2019
@wyardley
Copy link
Contributor

@bastelfreak unless they’ve been reworked, I think the acceptance tests still are only using the old version of RMQ and erlang?

I can double check, but if so, it’s actually a pretty non-trivial task to add.

@bastelfreak
Copy link
Member

ah, right.

@bastelfreak bastelfreak merged commit f8d2170 into voxpupuli:master Oct 12, 2019
@wyardley
Copy link
Contributor

Maybe an additional unit test would have been good though?

@paescuj paescuj mentioned this pull request Oct 29, 2019
@codeinthehole codeinthehole deleted the fix-version-regex branch April 28, 2020 13:10
wyardley added a commit to wyardley/puppet-rabbitmq that referenced this pull request Jul 31, 2022
- Update existing tests to include the leading spaces and some
  additional formatting
- Add new test case for the newer format included in `rabbitmqctl -q
  status` that's included in RabbitMQ >= v3.8.x, originally fixed in
  voxpupuli#814

See discussion in voxpupuli#872
wyardley added a commit that referenced this pull request Nov 14, 2024
- Update existing tests to include the leading spaces and some
  additional formatting
- Add new test case for the newer format included in `rabbitmqctl -q
  status` that's included in RabbitMQ >= v3.8.x, originally fixed in
  #814

See discussion in #872
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants