Re: [pve-devel] [PATCH manager 3/3] api: ceph: update return value definitions

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 21/12/2022 14:30, Aaron Lauterer wrote:
> to have at either a more accurate description or some description at
> all. For objects returning a lot of data, for example individual Ceph
> services, a full description has been omitted as I think that this would
> be a bit much.
> 
> Signed-off-by: Aaron Lauterer <a.lauterer@proxmox.com>
> ---
>  PVE/API2/Ceph.pm         |  7 ++++++-
>  PVE/API2/Ceph/MON.pm     | 14 +++++++++++---
>  PVE/API2/Ceph/OSD.pm     | 13 +++++++++++++
>  PVE/API2/Cluster/Ceph.pm | 23 ++++++++++++++++++++++-
>  4 files changed, 52 insertions(+), 5 deletions(-)
> 
> diff --git a/PVE/API2/Ceph.pm b/PVE/API2/Ceph.pm
> index 55220324..cc8720b2 100644
> --- a/PVE/API2/Ceph.pm
> +++ b/PVE/API2/Ceph.pm
> @@ -622,7 +622,12 @@ __PACKAGE__->register_method ({
>  	type => 'array',
>  	items => {
>  	    type => "object",
> -	    properties => {},
> +	    properties => {
> +		name => {
> +		    description => "Name of the CRUSH rule.",
> +		    type => "string",
> +		}
> +	    },
>  	},
>  	links => [ { rel => 'child', href => "{name}" } ],
>      },
> diff --git a/PVE/API2/Ceph/MON.pm b/PVE/API2/Ceph/MON.pm
> index 5771bb46..b452045c 100644
> --- a/PVE/API2/Ceph/MON.pm
> +++ b/PVE/API2/Ceph/MON.pm
> @@ -212,9 +212,17 @@ __PACKAGE__->register_method ({
>  	items => {
>  	    type => "object",
>  	    properties => {
> -		name => { type => 'string' },
> -		addr => { type => 'string', optional => 1 },
> -		host => { type => 'string', optional => 1 },
> +		name		    => { type => 'string' },
> +		addr		    => { type => 'string', optional => 1 },
> +		host		    => { type => 'boolean', optional => 1 },
> +		direxists	    => { type => 'string', optional => 1 },
> +		quorum		    => { type => 'boolean', optional => 1 },
> +		host		    => { type => 'string', optional => 1 },
> +		rank		    => { type => 'integer', optional => 1 },
> +		service		    => { type => 'integer', optional => 1 },
> +		state		    => { type => 'string', optional => 1 },
> +		ceph_version	    => { type => 'string', optional => 1 },
> +		ceph_version_short  => { type => 'string', optional => 1 },

please avoid that code formatting style! It looks very out of place in PVE
source and would require touching unrelated lines if a longer variant gets
added or the longest one gets removed, which is just a nuisance and messes
with git history without any real benefit.

>  	    },
>  	},
>  	links => [ { rel => 'child', href => "{name}" } ],
> diff --git a/PVE/API2/Ceph/OSD.pm b/PVE/API2/Ceph/OSD.pm
> index 93433b3a..ce1281d9 100644
> --- a/PVE/API2/Ceph/OSD.pm
> +++ b/PVE/API2/Ceph/OSD.pm
> @@ -88,6 +88,19 @@ __PACKAGE__->register_method ({
>      # fixme: return a list instead of extjs tree format ?
>      returns => {
>  	type => "object",
> +	items => {
> +	    type => "object",
> +	    properties => {
> +		flags => { type => "string" },
> +		root => {
> +		    type => "object",
> +		    description => "extjs formatted grid tree",

It's ExtJS, but actually not really sure why that is relevant here, the ExtJS
tree store format is a very simple and relatively common serialization format
for a tree.

> +		},
> +	    },
> +	},
> +
> +
> +	}
>      },
>      code => sub {
>  	my ($param) = @_;
> diff --git a/PVE/API2/Cluster/Ceph.pm b/PVE/API2/Cluster/Ceph.pm
> index 7f825003..49f84b24 100644
> --- a/PVE/API2/Cluster/Ceph.pm
> +++ b/PVE/API2/Cluster/Ceph.pm
> @@ -68,7 +68,20 @@ __PACKAGE__->register_method ({
>  	    },
>  	},
>      },
> -    returns => { type => 'object' },
> +    returns => {
> +	type => 'object'
> +	description => "Items for each type of service containing objects for each instance.",
> +	items => {
> +	    type => "object",
> +	    properties => {
> +		mds => { type => "object" },
> +		mgr => { type => "object" },
> +		mon => { type => "object" },
> +		node => { type => "object" },
> +		osd => { type => "object" },

description (e.g., it might not be clear that "node" holds the ceph version on
that node), optionality and maybe (some common/most-useful) sub-properties could
be great to have too.

> +	    }
> +	},
> +    },
>      code => sub {
>  	my ($param) = @_;
>  
> @@ -181,6 +194,14 @@ __PACKAGE__->register_method ({
>  		    description => "Flag name.",
>  		    type => 'string', enum => $possible_flags_list,
>  		},
> +		description => {
> +		    description => "Flag description.",
> +		    type => 'string',
> +		},
> +		value => {
> +		    description => "Flag value.",
> +		    type => 'boolean',
> +		},
>  	    },
>  	},
>  	links => [ { rel => 'child', href => "{name}" } ],