[ovirt-devel] Gluster Volume Snapshots - Feature review

[Juan Hernández]

On 12/11/2014 05:36 AM, Shubhendu Tripathi wrote:
> Hi Juan,
> 
> Incorporated the review comments.
> Kindly have a look and let us know if everything looks well and good.
> 
> Thanks and Regards,
> Shubhendu
> 

Thanks for making the changes. I have some additional comments:

1. URL segments shouldn't have underscores. For example, you are
proposing URLs like this:

  /clusters/{cluster:id}/glustervolumes/{volume:id}/volume_snapshots

The last component should be "volumesnapshots", without the underscore.
Note that on the other hand XML element should have underscores, like in
"<volume_snapshots>", that is correct.

2. Try to avoid abbreviations. For example, instead of "whatever_params"
use "whatever_parameters", and instead of "scheduling_det" use
"scheduling_details", use "cron_expression" instead of "cronexpr", so on.

3. If possible the action to delete a snapshot shouldn't receive any
parameters, not even an empty "<action/>" element.

4. The "schedulesnapshot" action should be modeled using REST style, not
an action. My understanding is that you plan to have for each volume a
set of rules that define when the snapshots will be created. This should
be implemented as a sub-collection of the volume, so that these rules
can be queried, added, modified and deleted:

  To query:

  GET ...{volume:id}/snapshotrules
  <snapshot_rules>
    <snapshot_rule id="..." href="...">
      <crontab_expression>...</crontab_expression>
    </snapshot_rule>
    ...
  </snapshot_rules>

  To add:

  POST ...{volume:id{/snapshotrules
  <snapshot_rule>
    <crontab_expression>...</crontab_expression>
  </snapshot_rule>

  To modify:

  PUT ...{volume:id}/snapshotrules/{snapshotrule:id}
  <snapshot_rule>
    <crontab_expression>...</crontab_expression>
  </snapshot_rule>

  To delete (note that there is no body):

   DELETE ...{volume:id}/snapshotrules/{snapshotrule:id}

If there will be only one of these rules per volume then you can model
them as attributes of the volume itself, without the sub-collection.

5. The "snapshotconfigs" should be modeled as an attribute of the
volume, not as an action.

> On 12/09/2014 07:23 PM, Shubhendu Tripathi wrote:
>> Thanks Juan for the comments. I would update the wiki accordingly and send for confirmation.
>>
>> Regards
>> Shubhendu
>>
>> Sent from Samsung Mobile
>>
>> -------- Original message --------
>> From: Juan Hernández <jhernand at redhat.com>
>> Date: 09/12/2014  18:51  (GMT+05:30)
>> To: Shubhendu Tripathi <shtripat at redhat.com>,devel at ovirt.org,Michael Pasternak <mpastern at redhat.com>
>> Subject: Re: [ovirt-devel] Gluster Volume Snapshots - Feature review
>>   
>> On 12/04/2014 07:11 PM, Shubhendu Tripathi wrote:
>>> Hi Juan/Michael,
>>>
>>> This is a gentle reminder for the review of the REST api design for the
>>> below feature.
>>> We would be starting the REST development for the same soon (probably by
>>> Dec 2014 end).
>>>
>>> If there are specific comments, please pass it on. If no comments we
>>> would go ahead with the current design and implement accordingly.
>>>
>>> Request your time for this.
>>>
>>> Thanks and Regards,
>>> Shubhendu
>>>
>>> On 11/10/2014 12:22 PM, Shubhendu Tripathi wrote:
>>>> Hi All,
>>>>
>>>> Please help us to review the design of Gluster Volume Snapshots in oVirt,
>>>>
>>>> Here are two design on wiki page
>>>>
>>>> General Feature Design
>>>> http://www.ovirt.org/Features/GlusterVolumeSnapshots
>>>>
>>>> Detailed Design
>>>> http://www.ovirt.org/Features/Design/GlusterVolumeSnapshots
>>>>
>>>> We target it in ovirt 3.6 release.
>>>>
>>>> Marked Juan/Michael specifically for REST review.
>>>>
>>>> Best Regards,
>>>> Shubhendu Tripathi
>> My comments about the RESTAPI:
>>
>> 1. You can't use the "snapshot" and "snapshots" XML elements, as those
>> are already in use for disk snapshots, and we don't have name space
>> support in the RESTAPI. You will have to use something different, for
>> example "gluster_volume_snapshots" and "gluster_volume_snapshot".
>>
>> 2. When adding a volume snapshot the name of the volume shouldn't be a
>> parameter, as that is implicit. Only the name and description of the
>> snapshot should be provided.
>>
>> 3. The operation to delete a snapshot should be performed on the
>> snapshot resource, not on the collection:
>>
>>    DELETE
>> /clusters/{cluster:id}/glustervolumes/{volume:id}/snapshots/{snapshot:id}
>>
>> Ideally this operation shouldn't receive any parameters (thus no body).
>> If it does require parameters then they should be contained inside an
>> "action" element.
>>
>> 4. The operation to update the snapshot configuration should be the PUT
>> operation of the volume, not a new "snapshotconfig" sub-resource, as
>> these kind of sub-resources aren't well supported by the SDKs and the CLI.
>>
> 
> _______________________________________________
> Devel mailing list
> Devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
> 


-- 
Dirección Comercial: C/Jose Bardasano Baos, 9, Edif. Gorbea 3, planta
3ºD, 28016 Madrid, Spain
Inscrita en el Reg. Mercantil de Madrid – C.I.F. B82657941 - Red Hat S.L.