Kanagaraj M has posted comments on this change.
Change subject: gluster: bll command for start/stop of services
......................................................................
Patch Set 3: (6 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/ManageGlusterServiceCommand.java
Line 35:
Line 36: protected Guid clusterId;
Line 37: protected Guid serverId;
Line 38: protected ServiceType serviceType;
Line 39: protected String actionType;
This could be extracted to an enum for easy use.
Line 40: protected List<String> errors = new ArrayList<String>();
Line 41:
Line 42: public static final String
MANAGE_GLUSTER_SERVICE_ACTION_TYPE_START = "start";
Line 43: public static final String MANAGE_GLUSTER_SERVICE_ACTION_TYPE_STOP
= "stop";
Line 84:
Line 85: if
(!actionType.equalsIgnoreCase(ManageGlusterServiceCommand.MANAGE_GLUSTER_SERVICE_ACTION_TYPE_START)
||
Line 86:
!actionType.equalsIgnoreCase(ManageGlusterServiceCommand.MANAGE_GLUSTER_SERVICE_ACTION_TYPE_STOP))
{
Line 87: return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_INVALID_ACTION_TYPE);
Line 88: }
this could be simplified as
if (!manageActionDetailsMap.keySet().contains(actionType)) {
return ..
}
Line 89:
Line 90: if (Guid.isNullOrEmpty(clusterId) &&
Guid.isNullOrEmpty(serverId)) {
Line 91: return
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CLUSTERID_AND_SERVERID_BOTH_NULL);
Line 92: }
Line 99: if (!(Guid.isNullOrEmpty(clusterId))) {
Line 100: performActionForServicesOfCluster();
Line 101: } else if (!(Guid.isNullOrEmpty(serverId))) {
Line 102: performActionForServicesOfServer();
Line 103: }
what if the user provides both cluterId and serverId?. So you could swap the
above comparisons.
Line 104: }
Line 105:
Line 106: private void performActionForServicesOfCluster() {
Line 107: List<VDS> servers =
getClusterUtils().getAllUpServers(clusterId);
Line 153: for (GlusterServerService service : serviceList) {
Line 154: if
(service.getServerId().equals(pairResult.getFirst().getId())
Line 155: && pairResult.getFirst().getStatus() !=
VDSStatus.Error) {
Line 156:
service.setStatus((actionType.equalsIgnoreCase(MANAGE_GLUSTER_SERVICE_ACTION_TYPE_START))
? GlusterServiceStatus.RUNNING
Line 157: : GlusterServiceStatus.STOPPED);
you can get the target status by
manageActionDetailsMap.get(actionType).getStatus()
Line 158: }
Line 159:
Line 160: getGlusterServerServiceDao().update(service);
Line 161: }
....................................................
File
frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/Enums.java
Line 340:
Line 341: String ServiceType___SHD();
Line 342:
Line 343: String GlusterServiceStatus___STARTED();
Line 344:
GlusterServiceStatus doesn't have an enum constant named "STARTED"
Line 345: String GlusterServiceStatus___STOPPED();
....................................................
File
frontend/webadmin/modules/uicompat/src/main/resources/org/ovirt/engine/ui/uicompat/Enums.properties
Line 148: GlusterVolumeType___STRIPE=Stripe
Line 149: GlusterVolumeType___DISTRIBUTED_STRIPE=Distributed Stripe
Line 150: GlusterStatus___UP=Up
Line 151: GlusterStatus___DOWN=Down
Line 152: GlusterServiceStatus___STARTED=Service started
same here.
Also in the ui everywhere we represent the statuses as UP/DOWN/<more>, so i
would be RUNNING to be mapped to UP and STOPPED to DOWN.
And add the translation for remaining items of GlusterServiceStatus as well.
This can be moved to LocalizedEnums if you want the translation to work on
other locales as well.
Line 153: GlusterServiceStatus___STOPPED=Service stopped
Line 154: TransportType___TCP=TCP
Line 155: TransportType___RDMA=RDMA
Line 156: ServiceType___NFS=NFS
--
To view, visit http://gerrit.ovirt.org/14831
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcab38866c49c6f5d43e3b33006c428ec9304501
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shubhendu Tripathi <shtri...@redhat.com>
Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com>
Gerrit-Reviewer: Sahina Bose <sab...@redhat.com>
Gerrit-Reviewer: Shubhendu Tripathi <shtri...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
