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 <jhern...@redhat.com>
Date: 09/12/2014 18:51 (GMT+05:30)
To: Shubhendu Tripathi <shtri...@redhat.com>,devel@ovirt.org,Michael Pasternak
<mpast...@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@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel
_______________________________________________
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel
