Yair Zaslavsky has posted comments on this change.
Change subject: gluster: bll command to start/stop/restart service
......................................................................
Patch Set 4: (3 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/gluster/ManageGlusterServiceCommand.java
Line 123: private void performActionForServicesOfCluster() {
Line 124: List<VDS> servers =
getClusterUtils().getAllUpServers(clusterId);
Line 125: final List<GlusterService> serviceList = getServiceList();
Line 126:
Line 127: List<Callable<Pair<VDS, VDSReturnValue>>> taskList = new
ArrayList<Callable<Pair<VDS, VDSReturnValue>>>();
Would be nicer if you extract this to a method of getTaskLIst, or even call it
in a different name, task is a bit misleading with vdsm task at this point.
Line 128: for (final VDS upServer : servers) {
Line 129: final List<String> serviceListStr = new
ArrayList<String>();
Line 130: for (GlusterService srvc : serviceList) {
Line 131: serviceListStr.add(srvc.getServiceName());
Line 159: } else {
Line 160: setSucceeded(false);
Line 161: }
Line 162:
Line 163: if (errors.size() > 0) {
i would move the errors.size() > 0 part to the block of !taskList.isEmpty() and
extract this block to a method of invokeXXX (but please, not "invokeTasks" - as
tasks can be confused with vdsm tasks).
Line 164: setSucceeded(false);
Line 165: handleVdsErrors(getAuditLogTypeValue(), errors);
Line 166: addCustomValue(GlusterConstants.FAILURE_MESSAGE,
StringUtils.join(errors, SystemUtils.LINE_SEPARATOR));
Line 167: } else {
Line 204: if
(serverService.getServiceId().equals(service.getId())
Line 205: && serverService.getServiceType() ==
serviceType
Line 206: && pairResult.getFirst().getStatus() !=
VDSStatus.Error) {
Line 207:
serverService.setStatus(manageActionDetailsMap.get(actionType).getStatus());
Line 208:
getGlusterServerServiceDao().update(serverService);
This will hurt performance - please see the Spring Batch patch on gerrit, I
hope we'll get it soon. If possible I would like to wait with merge until
Spring Batch patch gets in.
Line 209: break;
Line 210: }
Line 211: }
Line 212: }
--
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: 4
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Shubhendu Tripathi <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Kanagaraj M <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Shubhendu Tripathi <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
