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

Use JSON format to obtain data from 'ceph -s' #926

Closed
wants to merge 1 commit into from

Conversation

xavise
Copy link
Contributor

@xavise xavise commented Jul 25, 2018

'ceph -s' text output format strongly depends upon ceph version. Using JSON format to obtain data works across different ceph versions.

'ceph -s' text output format strongly depends upon ceph version. Using JSON format to obtain data works across different ceph versions.
@xavise xavise closed this Jul 25, 2018
@sumpfralle
Copy link
Collaborator

This change looks good! Thank you.

You closed the pull request: did you mean to retract it or did you want to indicate, that it is finished from your side?
(the usual github flow would be to close it only, if it is retracted or rejected)

@xavise
Copy link
Contributor Author

xavise commented Jul 25, 2018

Thank you for comments. I was about to include another optimization. I am going to open a new pull request right away.

@sumpfralle
Copy link
Collaborator

I am going to open a new pull request right away.

great!

Regarding the evaluation of the data: I think, it would be good to run ceph -s only once and store the result in a variable (instead of requesting it multiple times). This would prevent potential inconsistencies based on the delay between successive requests.
Just a hint - I am not sure, how relevant this is for this specific plugin ...

@xavise
Copy link
Contributor Author

xavise commented Jul 25, 2018

You're absolutely right. That's exactly the optimization I was talking about.

I have already opened a new pull request including it:
#927

Thanks again for your comments! :-)

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

Successfully merging this pull request may close these issues.

None yet

2 participants