CEPH filesystem development
 help / color / mirror / Atom feed
From: Stratos Psomadakis <psomas@grnet.gr>
To: Josh Durgin <josh.durgin@inktank.com>
Cc: Yehuda Sadeh <yehuda@inktank.com>,
	ceph-devel@vger.kernel.org, synnefo-devel@googlegroups.com
Subject: Re: [PATCH v2] rbd: Support plain/json/xml output formatting
Date: Mon, 14 Jan 2013 15:19:20 +0200	[thread overview]
Message-ID: <50F405D8.4000200@grnet.gr> (raw)
In-Reply-To: <50E76AAD.1040808@inktank.com>

[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]

On 01/05/2013 01:50 AM, Josh Durgin wrote:
> On 12/19/2012 04:17 AM, Stratos Psomadakis wrote:
>> This patch renames the --format option to --image-format, for
>> specyfing the RBD
>> image format, and uses --format to specify the output formating (to be
>> consistent with the other ceph tools). To avoid breaking backwards
>> compatibility
>> with existing scripts, rbd will still accept --format [1|2] for the
>> image
>> format, but will print a warning message, noting its use is deprecated.
>>
>> The rbd subcommands that support the new --format option are : ls,
>> info, snap
>> list, children, showmapped, lock list.
>>
>> Signed-off-by: Stratos Psomadakis <psomas@grnet.gr>
>> ---
>> Hi,
>>
>> this is the updated version of the patch. I renamed --format option to
>>   --image-format, and used --format to specify the output formatting,
>> as you
>> suggested.
>>
>> I also implemented some basic error checking on the --format input,
>> and modified
>> the rbd subcommands you mentioned, to support plain/json/xml output
>> formatting.
>> Although, I'm not sure if the json and xml output of those commands
>> is what
>> you'd want.
>>
>> The style issues should also be resolved now.
>>
>> Let me know what you think.
>
> There were still some style issues (missing braces, long lines, and
> spaces instead of tabs), but I squashed fixes to those into the
> wip-rbd-formatted-output branch.
>
> I changed the output a bit to uses ints, arrays, etc. where it
> seemed appropriate. The tests aren't quite done yet, but does
> this new json output look good to you?
>
> Josh

Hi,

Sorry for the delay. Thanks for resolving the style issues and cleaning
this up.

Wrt the json output, I think it's fine as it is now.

Just a minor comment. The --pretty-format option doesn't seem to be
documented (in either rbd --help or rbd man page). Other than that, I
don't see any other issues.

Thanks,
Stratos

-- 
Stratos Psomadakis
<psomas@grnet.gr>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

  reply	other threads:[~2013-01-14 13:19 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-16 14:36 rbd tool changed format? (breaks compatibility) Constantinos Venetsanopoulos
2012-11-16 17:14 ` Josh Durgin
2012-11-19 10:20   ` Constantinos Venetsanopoulos
2012-12-13 15:37   ` [PATCH] rbd: Add --json flag for the showmapped command Stratos Psomadakis
2012-12-13 17:17     ` Yehuda Sadeh
2012-12-14  8:57       ` Stratos Psomadakis
2012-12-17 17:32         ` Josh Durgin
2012-12-17 17:35           ` Yehuda Sadeh
2012-12-17 18:35             ` Josh Durgin
2012-12-17 17:25     ` Josh Durgin
2012-12-19 12:17       ` [PATCH v2] rbd: Support plain/json/xml output formatting Stratos Psomadakis
2013-01-04 23:50         ` Josh Durgin
2013-01-14 13:19           ` Stratos Psomadakis [this message]
2013-01-14 17:09             ` Josh Durgin

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=50F405D8.4000200@grnet.gr \
    --to=psomas@grnet.gr \
    --cc=ceph-devel@vger.kernel.org \
    --cc=josh.durgin@inktank.com \
    --cc=synnefo-devel@googlegroups.com \
    --cc=yehuda@inktank.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox