Shireesh Anjal has posted comments on this change.
Change subject: engine: Delete Gluster Volume
......................................................................
Patch Set 14: (4 inline comments)
responses to Michael's comments inline.
SS, since this patch is already merged, you should send another patch with
following changes:
- modify DeleteGlusterVolumeVDSCommand to extend from
AbstractGlusterBrokerCommand
- In DeleteGlusterVolumeCommand, if VDS command fails, populate
"executionFailedMessages" with the message from "vdsError"
Refer the "CreateGlusterVolume" BLL and VDS commands where this is already
implemented.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/DeleteGlusterVolumeCommand.java
Line 51: if(getSucceeded()) {
Not in all cases.
In case the VDS command fails because of an error from the "gluster" CLI, we
would like to capture it and make available to UI / REST api. This is handled
in the AbstractGlusterBrokerCommand, and all gluster related VDS commands
should extend from it. In this case, the errors are stored in the "vdsError"
field of the return value, setting setSucceeded to false. No exception is
thrown.
Since this patch is already merged, I suggest you send another patch modifying
DeleteGlusterVolumeVDSCommand to extend from AbstractGlusterBrokerCommand.
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VdcActionType.java
Line 264: DeleteGlusterVolume(1405, ActionGroup.DELETE_GLUSTER_VOLUME),
There was a discussion in engine-devel mailing list where it was recommended to
leave a comma before the semicolon, which supposedly makes life easier when
merging because one can introduce a new enum at the end without changing the
existing last line of the enum definition.
http://lists.ovirt.org/pipermail/engine-devel/2012-April/001315.html
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/vdscommands/VDSCommandType.java
Line 134: DeleteGlusterVolume("org.ovirt.engine.core.vdsbroker.gluster"),
There was a discussion in engine-devel mailing list where it was recommended to
leave a comma before the semicolon, which supposedly makes life easier when
merging because one can introduce a new enum at the end without changing the
existing last line of the enum definition.
http://lists.ovirt.org/pipermail/engine-devel/2012-April/001315.html
....................................................
File
frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
Line 682:
Some messages were added here in previous patch-sets, and then removed owing to
review comments. Looks like an empty line remained. Ideally this source should
have been removed from the patch-set. However I guess it's ok now, since the
patch is already merged.
--
To view, visit http://gerrit.ovirt.org/3744
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ide0d89c7100d787a4c66c29b2c1cc0832c494b33
Gerrit-PatchSet: 14
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Selvasundaram <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Dhandapani Gopal <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Livnat Peer <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Ori Liel <[email protected]>
Gerrit-Reviewer: Raksha Rao <[email protected]>
Gerrit-Reviewer: Selvasundaram <[email protected]>
Gerrit-Reviewer: Shireesh Anjal <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
