From: Aaron Lauterer <a.lauterer@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>,
Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] applied: Re: [PATCH v2 manager] pveceph: change status from long JSON to ceph -s
Date: Mon, 21 Dec 2020 15:31:12 +0100 [thread overview]
Message-ID: <549b7fa7-98b1-bacb-b3ec-33eef0453969@proxmox.com> (raw)
In-Reply-To: <46039410-6f9f-80da-8508-d48ffec96670@proxmox.com>
On 12/21/20 3:25 PM, Thomas Lamprecht wrote:
> On 21/12/2020 11:07, Aaron Lauterer wrote:
>> Printing a lot of very detailed JSON output on the CLI is not very
>> useful.
>>
>> Printing the `ceph -s` overview is much more suited to give an overview
>> of the ceph cluster status.
>>
>> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
>> ---
>> v1 -> v2:
>> * added check if Ceph is iniated to avoid ugly error msg
>
> good idea
>
>> * removed eval (if the command fails, we want to see the error msg)
>> * code style: avoiding long lines
>>
>> @thomas thx for the hint in the dev guidelines regarding long lines.
>> somehow I missed that until now and took the surrounding (older) code as
>> guideline :)
>>
>
> please also add a trailing comma to the last argument when doing all
> arguments on separate lines - at least if they're a "eats all" param hash
> or array, avoids the need to change multiple lines when adding or removing
> an argument. We may add a more specific rule for that though.
ah, forgot about that :/
>
>>
>> PVE/CLI/pveceph.pm | 26 +++++++++++++++++++++-----
>> 1 file changed, 21 insertions(+), 5 deletions(-)
>>
>>
>
> applied, thanks! Added a timeout of 15s (chose by gut feeling) to the
> run_command, as ceph -s hangs indefinitely if it cannot reach any monitor
> (and/or other ceph service).
thx!
prev parent reply other threads:[~2020-12-21 14:31 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-21 10:07 [pve-devel] " Aaron Lauterer
2020-12-21 14:25 ` [pve-devel] applied: " Thomas Lamprecht
2020-12-21 14:31 ` Aaron Lauterer [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=549b7fa7-98b1-bacb-b3ec-33eef0453969@proxmox.com \
--to=a.lauterer@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=t.lamprecht@proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.