Shireesh Anjal has posted comments on this change.
Change subject: engine: Gluster Peer Detach bll command
......................................................................
Patch Set 1: (8 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/GlusterHostRemoveCommand.java
Line 12: /**
Line 13: * BLL command for gluster peer detach
Line 14: */
Line 15: @NonTransactiveCommandAttribute
Line 16: public class GlusterHostRemoveCommand extends
GlusterCommandBase<GlusterHostRemoveParameters> {
I would like this to be called "RemoveGlusterServerCommand" as "server" is more
appropriate than "host" in gluster context.
Line 17:
Line 18: private static final long serialVersionUID = -3658615659129620366L;
Line 19:
Line 20: public GlusterHostRemoveCommand(GlusterHostRemoveParameters
params) {
....................................................
File
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/gluster/GlusterHostRemoveCommandTest.java
Line 42: assertTrue(cmd.canDoAction());
Line 43: }
Line 44:
Line 45: @Test
Line 46: public void canDoActionSucceedsWithForceAction() {
Since the canDoAction logic does not depend on the "forceAction" parameter, I
don't see why we need two separate test cases for canDoAction depending on
value of forceAction.
Line 47: cmd = spy(new GlusterHostRemoveCommand(new
GlusterHostRemoveParameters(SERVER_NAME, true)));
Line 48: prepareMocks(cmd);
Line 49: assertTrue(cmd.canDoAction());
Line 50: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/gluster/GlusterHostRemoveParameters.java
Line 5: /**
Line 6: * Parameter class with hostname and forceAction as parameters. <br>
Line 7: * This will be used by gluster host remove command. <br>
Line 8: */
Line 9: public class GlusterHostRemoveParameters extends
VdcActionParametersBase {
I would like this to be called RemoveGlusterServerParameters
Line 10: private static final long serialVersionUID = -1224829720081853632L;
Line 11:
Line 12: private String hostName;
Line 13: private boolean forceAction;
Line 8: */
Line 9: public class GlusterHostRemoveParameters extends
VdcActionParametersBase {
Line 10: private static final long serialVersionUID = -1224829720081853632L;
Line 11:
Line 12: private String hostName;
How about calling this as hostnameOrIp since both would work?
Line 13: private boolean forceAction;
Line 14:
Line 15: public GlusterHostRemoveParameters(String hostName, boolean
forceAction) {
Line 16: setHostName(hostName);
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java
Line 207: GLUSTER_VOLUME_REPLACE_BRICK_START(4017),
Line 208: GLUSTER_VOLUME_REPLACE_BRICK_START_FAILED(4018),
Line 209: GLUSTER_VOLUME_ADD_BRICK(4019),
Line 210: GLUSTER_VOLUME_ADD_BRICK_FAILED(4020),
Line 211: GLUSTER_HOST_REMOVE(4021),
Why not just add it in the end and avoid changing the other lines?
Line 212: GLUSTER_HOST_REMOVE_FAILED(4022),
Line 213: GLUSTER_VOLUME_PROFILE_START(4023),
Line 214: GLUSTER_VOLUME_PROFILE_START_FAILED(4024),
Line 215: GLUSTER_VOLUME_PROFILE_STOP(4025),
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/VdcBllMessages.java
Line 24: // Gluster types
Line 25: VAR__TYPE__GLUSTER_VOLUME,
Line 26: VAR__TYPE__GLUSTER_VOLUME_OPTION,
Line 27: VAR__TYPE__GLUSTER_BRICK,
Line 28: VAR__TYPE__GLUSTER_HOST,
Please change all references of "Gluster Host" to "Gluster Server"
Line 29:
Line 30: VAR__ACTION__RUN,
Line 31: VAR__ACTION__REMOVE,
Line 32: VAR__ACTION__ADD,
....................................................
File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
Line 740: # Gluster Error Messages
Line 741: VAR__TYPE__GLUSTER_VOLUME=$type Gluster Volume
Line 742: VAR__TYPE__GLUSTER_VOLUME_OPTION=$type Gluster Volume Option
Line 743: VAR__TYPE__GLUSTER_BRICK=$type Gluster Brick
Line 744: VAR__TYPE__GLUSTER_HOST=$type Gluster Host
Please change from HOST/Host to SERVER/Server
Line 745: VALIDATION.GLUSTER.VOLUME.ID.NOT_NULL=Volume ID is required.
Line 746: VALIDATION.GLUSTER.VOLUME.CLUSTER_ID.NOT_NULL=Cluster ID is required.
Line 747: VALIDATION.GLUSTER.VOLUME.NAME.NOT_NULL=Volume Name is required.
Line 748: VALIDATION.GLUSTER.VOLUME.TYPE.NOT_NULL=Volume Type is required.
....................................................
File
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
Line 730: @DefaultStringValue("$type Gluster Brick")
Line 731: String VAR__TYPE__GLUSTER_BRICK();
Line 732:
Line 733: @DefaultStringValue("$type Gluster Host")
Line 734: String VAR__TYPE__GLUSTER_HOST();
Please change host to server
Line 735:
Line 736: @DefaultStringValue("Cannot ${action} ${type}. The chosen disk
drive letter is already in use, please select a free one.")
Line 737: String ACTION_TYPE_FAILED_DISK_LETTER_ALREADY_IN_USE();
Line 738:
--
To view, visit http://gerrit.ovirt.org/9044
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I171f1b6eb9d01f1ffb2b4a0be52a15887f534c2d
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dhandapani Gopal <dgo...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Shireesh Anjal <san...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
