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

skip data collection on / vhost to address issue #32 #62

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tnibert
Copy link

@tnibert tnibert commented Aug 30, 2017

This may not be a good candidate to merge, but it does resolve issue #32 401 returned when using plugin - even though standard Basic Auth curls work using creds, so hopefully at least contributes to a better and less hacky solution.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 99.693% when pulling 02769fc on tnibert:master into b6e3e59 on NYTimes:master.

@mrunge
Copy link
Contributor

mrunge commented Sep 1, 2017

The change itself looks ok for me, even if it reduces coverage.

@supermitch
Copy link

This pull request means you can never get stats on "/" vhost. My understanding is that vhost prefix (when configuring RabbitMQ) is optional, so what if I want to use the default? This code would never allow me to get stats on "/"?

@tnibert
Copy link
Author

tnibert commented May 6, 2018

Yes I think that is probably true. I just ran into the issue #32 in my own application and this is the quick hack that I used for it to work with my application. I submitted this pull request mostly just as a pointer like "look here, clue to the root cause."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants