You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by Prasanna Santhanam <ts...@apache.org> on 2013/06/04 10:57:40 UTC

Re: StoragePoolForMigrationResponse and StoragePoolResponse

On Fri, May 31, 2013 at 06:28:39PM +0530, Prasanna Santhanam wrote:
> On Fri, May 31, 2013 at 12:24:20PM +0000, Pranav Saxena wrote:
> > Hey Prasanna , 
> > 
> > I see that the response  object name is
> > findstoragepoolsformigrationresponse , which is correct as shown
> > below .  Are you referring to this API or something else  ?
> > 
> > http://MSIP:8096/client/api?command=findStoragePoolsForMigration
> > 
> > <findstoragepoolsformigrationresponse
> > cloud-stack-version="4.2.0-SNAPSHOT">
> > 
> >  </findstoragepoolsformigrationresponse>
> > 
> 
> No that's what is shown to the user. I meant the class within
> org.apache.cloudstack.api.response
> 
Fixed with 0401774a09483354f5b8532a30943351755da93f

-- 
Prasanna.,

------------------------
Powered by BigRock.com


Re: StoragePoolForMigrationResponse and StoragePoolResponse

Posted by Nitin Mehta <Ni...@citrix.com>.
Ideally, we shouldn't be muddling the storage pool response here for this
attribute. We should try and move away from including all the various
attributes for the resource and make it one huge monolithic response. We
should include only the first class attributes, say the name, URI etc. in
the storagePoolResponse.

Also to list other attributes which do not make it in the first class
attribute list, we should design a new response which is generic enough to
take in additional attributes added, may be something like
storagePoolDetailsResponse.

Thanks,
-Nitin

On 07/06/13 10:35 PM, "Min Chen" <mi...@citrix.com> wrote:

>Maybe attribute is not accurate in this sense, but it is just some
>metadata related to a storage pool. Just like tags, or statistics (For
>example, AccountResponse), display to user flag (for example, displayVm in
>UserVMResponse) we have created for any other CloudStack Entity. We didn't
>need to create a different Response class just to include or exclude this
>information to make it over complicated.
>
>Thanks
>-min
>
>On 6/6/13 11:33 PM, "Devdeep Singh" <de...@citrix.com> wrote:
>
>>suitableformigration isn't an attribute of the storage pool. It just
>>tells whether a particular pool is suitable for migrating a particular
>>volume. For example, if volume A has to be migrated to another pool, then
>>the pools available are listed and if the tags on the pool and volume do
>>not match then it is flagged as unsuitable. For another volume it may be
>>flagged suitable. So it really isn't an attribute of a storage pool and I
>>believe it doesn't belong in the StoragePoolResponse object.
>>
>>Regards,
>>Devdeep
>>
>>> -----Original Message-----
>>> From: Min Chen [mailto:min.chen@citrix.com]
>>> Sent: Friday, June 07, 2013 2:20 AM
>>> To: dev@cloudstack.apache.org
>>> Subject: Re: StoragePoolForMigrationResponse and StoragePoolResponse
>>> 
>>> I agree with Prasanna on this. We don't need to introduce several
>>>Storage
>>> pool related responses just for some specific apis. In some way,
>>> suitableFormigration is some kind of attribute that can be set on a
>>>storage
>>> pool or not. If you don't want to show it to listStoragePool call, you
>>>can set that
>>> as null so that json serialization will ignore it.
>>> 
>>> Just my two cents.
>>> -min
>>> 
>>> On 6/6/13 5:07 AM, "Devdeep Singh" <de...@citrix.com> wrote:
>>> 
>>> >Hi,
>>> >
>>> >StoragePoolResponse should really only be used for listing storage
>>>pools.
>>> >Putting a suitableformigration flag etc. makes it weird for other
>>>apis.
>>> >If tomorrow the response object is updated to include more statistics
>>> >for admin user to make a better decision, then such information gets
>>> >pushed in there which makes it unnatural for apis that just need the
>>> >list of storage pools. I am planning to update
>>> >StoragePoolForMigrationResponse to include the StoragePoolResponse
>>> >object and any other flag; suitableformigration in this case. I'll
>>>file a bug for
>>> the same.
>>> >
>>> >Regards,
>>> >Devdeep
>>> >
>>> >> -----Original Message-----
>>> >> From: Prasanna Santhanam [mailto:tsp@apache.org]
>>> >> Sent: Tuesday, June 04, 2013 2:28 PM
>>> >> To: dev@cloudstack.apache.org
>>> >> Subject: Re: StoragePoolForMigrationResponse and StoragePoolResponse
>>> >>
>>> >> On Fri, May 31, 2013 at 06:28:39PM +0530, Prasanna Santhanam wrote:
>>> >> > On Fri, May 31, 2013 at 12:24:20PM +0000, Pranav Saxena wrote:
>>> >> > > Hey Prasanna ,
>>> >> > >
>>> >> > > I see that the response  object name is
>>> >> > > findstoragepoolsformigrationresponse , which is correct as shown
>>> >> > > below .  Are you referring to this API or something else  ?
>>> >> > >
>>> >> > > http://MSIP:8096/client/api?command=findStoragePoolsForMigration
>>> >> > >
>>> >> > > <findstoragepoolsformigrationresponse
>>> >> > > cloud-stack-version="4.2.0-SNAPSHOT">
>>> >> > >
>>> >> > >  </findstoragepoolsformigrationresponse>
>>> >> > >
>>> >> >
>>> >> > No that's what is shown to the user. I meant the class within
>>> >> > org.apache.cloudstack.api.response
>>> >> >
>>> >> Fixed with 0401774a09483354f5b8532a30943351755da93f
>>> >>
>>> >> --
>>> >> Prasanna.,
>>> >>
>>> >> ------------------------
>>> >> Powered by BigRock.com
>>> >
>>
>


Re: StoragePoolForMigrationResponse and StoragePoolResponse

Posted by Min Chen <mi...@citrix.com>.
Maybe attribute is not accurate in this sense, but it is just some
metadata related to a storage pool. Just like tags, or statistics (For
example, AccountResponse), display to user flag (for example, displayVm in
UserVMResponse) we have created for any other CloudStack Entity. We didn't
need to create a different Response class just to include or exclude this
information to make it over complicated.

Thanks
-min

On 6/6/13 11:33 PM, "Devdeep Singh" <de...@citrix.com> wrote:

>suitableformigration isn't an attribute of the storage pool. It just
>tells whether a particular pool is suitable for migrating a particular
>volume. For example, if volume A has to be migrated to another pool, then
>the pools available are listed and if the tags on the pool and volume do
>not match then it is flagged as unsuitable. For another volume it may be
>flagged suitable. So it really isn't an attribute of a storage pool and I
>believe it doesn't belong in the StoragePoolResponse object.
>
>Regards,
>Devdeep
>
>> -----Original Message-----
>> From: Min Chen [mailto:min.chen@citrix.com]
>> Sent: Friday, June 07, 2013 2:20 AM
>> To: dev@cloudstack.apache.org
>> Subject: Re: StoragePoolForMigrationResponse and StoragePoolResponse
>> 
>> I agree with Prasanna on this. We don't need to introduce several
>>Storage
>> pool related responses just for some specific apis. In some way,
>> suitableFormigration is some kind of attribute that can be set on a
>>storage
>> pool or not. If you don't want to show it to listStoragePool call, you
>>can set that
>> as null so that json serialization will ignore it.
>> 
>> Just my two cents.
>> -min
>> 
>> On 6/6/13 5:07 AM, "Devdeep Singh" <de...@citrix.com> wrote:
>> 
>> >Hi,
>> >
>> >StoragePoolResponse should really only be used for listing storage
>>pools.
>> >Putting a suitableformigration flag etc. makes it weird for other apis.
>> >If tomorrow the response object is updated to include more statistics
>> >for admin user to make a better decision, then such information gets
>> >pushed in there which makes it unnatural for apis that just need the
>> >list of storage pools. I am planning to update
>> >StoragePoolForMigrationResponse to include the StoragePoolResponse
>> >object and any other flag; suitableformigration in this case. I'll
>>file a bug for
>> the same.
>> >
>> >Regards,
>> >Devdeep
>> >
>> >> -----Original Message-----
>> >> From: Prasanna Santhanam [mailto:tsp@apache.org]
>> >> Sent: Tuesday, June 04, 2013 2:28 PM
>> >> To: dev@cloudstack.apache.org
>> >> Subject: Re: StoragePoolForMigrationResponse and StoragePoolResponse
>> >>
>> >> On Fri, May 31, 2013 at 06:28:39PM +0530, Prasanna Santhanam wrote:
>> >> > On Fri, May 31, 2013 at 12:24:20PM +0000, Pranav Saxena wrote:
>> >> > > Hey Prasanna ,
>> >> > >
>> >> > > I see that the response  object name is
>> >> > > findstoragepoolsformigrationresponse , which is correct as shown
>> >> > > below .  Are you referring to this API or something else  ?
>> >> > >
>> >> > > http://MSIP:8096/client/api?command=findStoragePoolsForMigration
>> >> > >
>> >> > > <findstoragepoolsformigrationresponse
>> >> > > cloud-stack-version="4.2.0-SNAPSHOT">
>> >> > >
>> >> > >  </findstoragepoolsformigrationresponse>
>> >> > >
>> >> >
>> >> > No that's what is shown to the user. I meant the class within
>> >> > org.apache.cloudstack.api.response
>> >> >
>> >> Fixed with 0401774a09483354f5b8532a30943351755da93f
>> >>
>> >> --
>> >> Prasanna.,
>> >>
>> >> ------------------------
>> >> Powered by BigRock.com
>> >
>


RE: StoragePoolForMigrationResponse and StoragePoolResponse

Posted by Devdeep Singh <de...@citrix.com>.
suitableformigration isn't an attribute of the storage pool. It just tells whether a particular pool is suitable for migrating a particular volume. For example, if volume A has to be migrated to another pool, then the pools available are listed and if the tags on the pool and volume do not match then it is flagged as unsuitable. For another volume it may be flagged suitable. So it really isn't an attribute of a storage pool and I believe it doesn't belong in the StoragePoolResponse object.

Regards,
Devdeep

> -----Original Message-----
> From: Min Chen [mailto:min.chen@citrix.com]
> Sent: Friday, June 07, 2013 2:20 AM
> To: dev@cloudstack.apache.org
> Subject: Re: StoragePoolForMigrationResponse and StoragePoolResponse
> 
> I agree with Prasanna on this. We don't need to introduce several Storage
> pool related responses just for some specific apis. In some way,
> suitableFormigration is some kind of attribute that can be set on a storage
> pool or not. If you don't want to show it to listStoragePool call, you can set that
> as null so that json serialization will ignore it.
> 
> Just my two cents.
> -min
> 
> On 6/6/13 5:07 AM, "Devdeep Singh" <de...@citrix.com> wrote:
> 
> >Hi,
> >
> >StoragePoolResponse should really only be used for listing storage pools.
> >Putting a suitableformigration flag etc. makes it weird for other apis.
> >If tomorrow the response object is updated to include more statistics
> >for admin user to make a better decision, then such information gets
> >pushed in there which makes it unnatural for apis that just need the
> >list of storage pools. I am planning to update
> >StoragePoolForMigrationResponse to include the StoragePoolResponse
> >object and any other flag; suitableformigration in this case. I'll file a bug for
> the same.
> >
> >Regards,
> >Devdeep
> >
> >> -----Original Message-----
> >> From: Prasanna Santhanam [mailto:tsp@apache.org]
> >> Sent: Tuesday, June 04, 2013 2:28 PM
> >> To: dev@cloudstack.apache.org
> >> Subject: Re: StoragePoolForMigrationResponse and StoragePoolResponse
> >>
> >> On Fri, May 31, 2013 at 06:28:39PM +0530, Prasanna Santhanam wrote:
> >> > On Fri, May 31, 2013 at 12:24:20PM +0000, Pranav Saxena wrote:
> >> > > Hey Prasanna ,
> >> > >
> >> > > I see that the response  object name is
> >> > > findstoragepoolsformigrationresponse , which is correct as shown
> >> > > below .  Are you referring to this API or something else  ?
> >> > >
> >> > > http://MSIP:8096/client/api?command=findStoragePoolsForMigration
> >> > >
> >> > > <findstoragepoolsformigrationresponse
> >> > > cloud-stack-version="4.2.0-SNAPSHOT">
> >> > >
> >> > >  </findstoragepoolsformigrationresponse>
> >> > >
> >> >
> >> > No that's what is shown to the user. I meant the class within
> >> > org.apache.cloudstack.api.response
> >> >
> >> Fixed with 0401774a09483354f5b8532a30943351755da93f
> >>
> >> --
> >> Prasanna.,
> >>
> >> ------------------------
> >> Powered by BigRock.com
> >


Re: StoragePoolForMigrationResponse and StoragePoolResponse

Posted by Min Chen <mi...@citrix.com>.
I agree with Prasanna on this. We don't need to introduce several Storage
pool related responses just for some specific apis. In some way,
suitableFormigration is some kind of attribute that can be set on a
storage pool or not. If you don't want to show it to listStoragePool call,
you can set that as null so that json serialization will ignore it.

Just my two cents.
-min

On 6/6/13 5:07 AM, "Devdeep Singh" <de...@citrix.com> wrote:

>Hi,
>
>StoragePoolResponse should really only be used for listing storage pools.
>Putting a suitableformigration flag etc. makes it weird for other apis.
>If tomorrow the response object is updated to include more statistics for
>admin user to make a better decision, then such information gets pushed
>in there which makes it unnatural for apis that just need the list of
>storage pools. I am planning to update StoragePoolForMigrationResponse to
>include the StoragePoolResponse object and any other flag;
>suitableformigration in this case. I'll file a bug for the same.
>
>Regards,
>Devdeep
>
>> -----Original Message-----
>> From: Prasanna Santhanam [mailto:tsp@apache.org]
>> Sent: Tuesday, June 04, 2013 2:28 PM
>> To: dev@cloudstack.apache.org
>> Subject: Re: StoragePoolForMigrationResponse and StoragePoolResponse
>> 
>> On Fri, May 31, 2013 at 06:28:39PM +0530, Prasanna Santhanam wrote:
>> > On Fri, May 31, 2013 at 12:24:20PM +0000, Pranav Saxena wrote:
>> > > Hey Prasanna ,
>> > >
>> > > I see that the response  object name is
>> > > findstoragepoolsformigrationresponse , which is correct as shown
>> > > below .  Are you referring to this API or something else  ?
>> > >
>> > > http://MSIP:8096/client/api?command=findStoragePoolsForMigration
>> > >
>> > > <findstoragepoolsformigrationresponse
>> > > cloud-stack-version="4.2.0-SNAPSHOT">
>> > >
>> > >  </findstoragepoolsformigrationresponse>
>> > >
>> >
>> > No that's what is shown to the user. I meant the class within
>> > org.apache.cloudstack.api.response
>> >
>> Fixed with 0401774a09483354f5b8532a30943351755da93f
>> 
>> --
>> Prasanna.,
>> 
>> ------------------------
>> Powered by BigRock.com
>


RE: StoragePoolForMigrationResponse and StoragePoolResponse

Posted by Devdeep Singh <de...@citrix.com>.
Hi,

StoragePoolResponse should really only be used for listing storage pools. Putting a suitableformigration flag etc. makes it weird for other apis. If tomorrow the response object is updated to include more statistics for admin user to make a better decision, then such information gets pushed in there which makes it unnatural for apis that just need the list of storage pools. I am planning to update StoragePoolForMigrationResponse to include the StoragePoolResponse object and any other flag; suitableformigration in this case. I'll file a bug for the same.

Regards,
Devdeep

> -----Original Message-----
> From: Prasanna Santhanam [mailto:tsp@apache.org]
> Sent: Tuesday, June 04, 2013 2:28 PM
> To: dev@cloudstack.apache.org
> Subject: Re: StoragePoolForMigrationResponse and StoragePoolResponse
> 
> On Fri, May 31, 2013 at 06:28:39PM +0530, Prasanna Santhanam wrote:
> > On Fri, May 31, 2013 at 12:24:20PM +0000, Pranav Saxena wrote:
> > > Hey Prasanna ,
> > >
> > > I see that the response  object name is
> > > findstoragepoolsformigrationresponse , which is correct as shown
> > > below .  Are you referring to this API or something else  ?
> > >
> > > http://MSIP:8096/client/api?command=findStoragePoolsForMigration
> > >
> > > <findstoragepoolsformigrationresponse
> > > cloud-stack-version="4.2.0-SNAPSHOT">
> > >
> > >  </findstoragepoolsformigrationresponse>
> > >
> >
> > No that's what is shown to the user. I meant the class within
> > org.apache.cloudstack.api.response
> >
> Fixed with 0401774a09483354f5b8532a30943351755da93f
> 
> --
> Prasanna.,
> 
> ------------------------
> Powered by BigRock.com