[ovirt-devel] Gluster Volume Snapshots - Feature review
Shubhendu Tripathi
shtripat at redhat.com
Thu Dec 18 13:08:01 UTC 2014
Hi Juan,
Thanks for the detailed comments.
My comments inline.
Thanks and Regards,
Shubhendu
On 12/11/2014 08:56 PM, Juan Hernández wrote:
> 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.
Sure. Will do the changes accordingly.
>
> 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.
Sure. Will do the changes accordingly.
>
> 3. If possible the action to delete a snapshot shouldn't receive any
> parameters, not even an empty "<action/>" element.
Sure. Will do the changes accordingly.
>
> 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.
At a time there would be only one rule for a volume, so as suggested it
could be a volume attribute. Will model accordingly.
>
> 5. The "snapshotconfigs" should be modeled as an attribute of the
> volume, not as an action.
As above.
>
>> 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
>>
>
More information about the Devel
mailing list