Skip to content
This repository has been archived by the owner on Nov 17, 2020. It is now read-only.

rabbitmqadmin get doesn't provide a way to specify the requeue parameter required by HTTP API in 3.7.x #574

Closed

Conversation

valleedelisle
Copy link

@valleedelisle valleedelisle commented May 22, 2018

Proposed Changes

We can't specify the requeue argument, and when we don't specify, rabbitmqadmin refuses it.

# $RMQADM get queue=blabla
*** {u'key_missing': u'requeue'}
# $RMQADM get queue=blabla requeue=true

ERROR: Argument "requeue" not recognised

rabbitmqadmin --help for help

Types of Changes

  • Bugfix (non-breaking change which fixes issue )
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (correction or otherwise)
  • Cosmetics (whitespace, appearance)

Checklist

  • I have read the CONTRIBUTING.md document
  • I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
  • All tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in related repositories

Further Comments

First time contributor, sorry if it's not perfect.

@pivotal-issuemaster
Copy link

@valleedelisle Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@valleedelisle valleedelisle changed the title Fixes the get queue bug, issue #573 Fixes the get queue bug May 22, 2018
@pivotal-issuemaster
Copy link

@valleedelisle Thank you for signing the Contributor License Agreement!

@michaelklishin
Copy link
Member

Shouldn't we expose the requeue parameter then? Simply forcing a default sounds like a workaround at best.

@michaelklishin
Copy link
Member

But thank you for taking the time to contribute and it's a good start, I'll look into adding support for the requeue parameter.

@michaelklishin michaelklishin changed the title Fixes the get queue bug rabbitmqadmin get doesn't provide a way to specify the requeue parameter required by HTTP API in 3.7.x May 22, 2018
@valleedelisle
Copy link
Author

Well having the default set to true is good in my opinion. If people are using the tool to troubleshoot, they probably don't want to consume the messages. Also, having the requeue defined there doesn't prevent the user from setting it to false using the argument.

But if you have a better idea, I can't wait to see, I'm à bit new to all this.

Thanks

@michaelklishin
Copy link
Member

michaelklishin commented May 23, 2018

I did a bit of research and we won't be accepting this PR. Here's why.

As of 3.7.0, basic.get supports more options via HTTP API. This means that the requeue option is gone and was replaced by ackmode, which does have a default (ack_requeue_true, same as management UI as of #542).

This PR tries to reinstall support for pre-3.7.0 HTTP API. We won't be doing that. Please use a rabbitmqadmin from 3.6.x against 3.6.x nodes and the argument mentioned above.

@michaelklishin
Copy link
Member

FTR, that change is mentioned in the change log under the management plugin section.

@michaelklishin
Copy link
Member

michaelklishin commented May 23, 2018

Thank you for taking the time to contribute, of course 👍.

michaelklishin added a commit to rabbitmq/rabbitmq-website that referenced this pull request May 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants