Alissa Bonas has posted comments on this change.
Change subject: core: restapi: add GetVmOvfConifugrationQuery
......................................................................
Patch Set 1: (4 inline comments)
I recommend adding unitests where possible.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetVmOvfConfigurationBySnapshotQuery.java
Line 12: }
Line 13:
Line 14: @Override
Line 15: protected void executeQueryCommand() {
Line 16: Snapshot snapshot =
getSnapshotDao().get(getParameters().getId(), getUserID(),
getParameters().isFiltered());
this code seems very similar to the one in GetVmConfigurationBySnapshotQuery
(isn't the same dao method called?).
why is there a need in a new query - what is different?
or at least some code reuse will be suitable here...
Line 17: if (snapshot != null) {
Line 18:
getQueryReturnValue().setReturnValue(snapshot.getVmConfiguration());
Line 19: }
Line 20: }
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendSnapshotResource.java
Line 47: return entity;
Line 48: }
Line 49:
Line 50: public Guid getSnapshotId() {
Line 51: return guid;
where is this class member defined?
Line 52: }
Line 53:
Line 54: @Override
Line 55: public Response restore(Action action) {
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendSnapshotSnapshotOvfResource.java
Line 11: }
Line 12:
Line 13: @Override
Line 14: public String get() {
Line 15: return parent.getEntity(String.class,
VdcQueryType.GetVmOvfConfigurationBySnapshot, new
IdQueryParameters(parent.getSnapshotId()), null);
what happens if there is no ovf configuration - what does the user get? 200 OK?
error?
Line 16: }
....................................................
Commit Message
Line 3: AuthorDate: 2013-06-26 09:38:23 +0300
Line 4: Commit: Liron Aravot <[email protected]>
Line 5: CommitDate: 2013-06-27 12:13:57 +0300
Line 6:
Line 7: core: restapi: add GetVmOvfConifugrationQuery
probably better change it to : core,restapi.
also, there's a typo in GetVmOvfConfigurationQuery - it's written Conifugration
Line 8:
Line 9: This patch adds the GetVmOvfConfigurationBySnapshotQuery query which
Line 10: returns the Ovf configuration of snapshot (if present).
Line 11:
--
To view, visit http://gerrit.ovirt.org/16176
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I46340c4461b57a4c314fb50ca9c19ac5fd08a451
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Deepak C Shetty <[email protected]>
Gerrit-Reviewer: Liron Ar <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
