Kanagaraj M has posted comments on this change. Change subject: webadmin : Gluster Volume Profile ......................................................................
Patch Set 29: (14 comments) http://gerrit.ovirt.org/#/c/27470/29/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/volumes/VolumeListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/volumes/VolumeListModel.java: Line 173: setStopCommand(new UICommand("Stop", this)); //$NON-NLS-1$ Line 174: setStartRebalanceCommand(new UICommand("StartRebalance", this)); //$NON-NLS-1$ Line 175: setStopRebalanceCommand(new UICommand("StopRebalace", this)); //$NON-NLS-1$ Line 176: setStatusRebalanceCommand(new UICommand("StatusRebalance", this)); //$NON-NLS-1$ Line 177: setStartVolumeProfilingCommand(new UICommand("Start Profiling", this));//$NON-NLS-1$ Remove space "StartProfiling" Line 178: setShowVolumeProfileDetailsCommand(new UICommand("Show Profile Details", this));//$NON-NLS-1$ Line 179: setStopVolumeProfilingCommand(new UICommand("Stop Profiling", this));//$NON-NLS-1$ Line 180: setOptimizeForVirtStoreCommand(new UICommand("OptimizeForVirtStore", this)); //$NON-NLS-1$ Line 181: Line 175: setStopRebalanceCommand(new UICommand("StopRebalace", this)); //$NON-NLS-1$ Line 176: setStatusRebalanceCommand(new UICommand("StatusRebalance", this)); //$NON-NLS-1$ Line 177: setStartVolumeProfilingCommand(new UICommand("Start Profiling", this));//$NON-NLS-1$ Line 178: setShowVolumeProfileDetailsCommand(new UICommand("Show Profile Details", this));//$NON-NLS-1$ Line 179: setStopVolumeProfilingCommand(new UICommand("Stop Profiling", this));//$NON-NLS-1$ same here Line 180: setOptimizeForVirtStoreCommand(new UICommand("OptimizeForVirtStore", this)); //$NON-NLS-1$ Line 181: Line 182: getRemoveVolumeCommand().setIsExecutionAllowed(false); Line 183: getStartCommand().setIsExecutionAllowed(false); Line 413: if (volume.getStatus() == GlusterStatus.UP) { Line 414: allowStart = false; Line 415: allowRemove = false; Line 416: allowStartProfiling = true; Line 417: allowStopProfiling = true; What happens if user selects 5 volumes, and 3 volumes down out of these 5 Line 418: } Line 419: else if (volume.getStatus() == GlusterStatus.DOWN) { Line 420: allowStop = false; Line 421: allowStartRebalance = false; Line 534: } else if(command.equals(getShowVolumeProfileDetailsCommand()) || command.getName().equals("Show Profile Details")) {//$NON-NLS-1$ Line 535: showVolumeProfiling(); Line 536: }else if(command.getName().equalsIgnoreCase("close_profile_stats")) {//$NON-NLS-1$ Line 537: setWindow(null); Line 538: } else if(command.getName().equalsIgnoreCase("profilingNotStarted")) {//$NON-NLS-1$ please be consistent with the command names. You could just use CamelCase Line 539: if(getConfirmWindow() == null) { Line 540: return; Line 541: } Line 542: getConfirmWindow().stopProgress(); Line 539: if(getConfirmWindow() == null) { Line 540: return; Line 541: } Line 542: getConfirmWindow().stopProgress(); Line 543: setConfirmWindow(null); move this block to a separate method Line 544: } Line 545: } Line 546: Line 547: private void startVolumeProfiling() { Line 547: private void startVolumeProfiling() { Line 548: if(getSelectedItems() == null) { Line 549: return; Line 550: } Line 551: GlusterVolumeParameters parameter = new GlusterVolumeParameters(((GlusterVolumeEntity) getSelectedItem()).getId()); What if user has selected multiple volumes Line 552: Frontend.getInstance().runAction(VdcActionType.StartGlusterVolumeProfile, parameter); Line 553: } Line 554: Line 555: private void stopVolumeProfiling() { Line 555: private void stopVolumeProfiling() { Line 556: if(getSelectedItems() == null) { Line 557: return; Line 558: } Line 559: GlusterVolumeParameters parameter = new GlusterVolumeParameters(((GlusterVolumeEntity) getSelectedItem()).getId()); same here Line 560: Frontend.getInstance().runAction(VdcActionType.StopGlusterVolumeProfile, parameter); Line 561: } Line 562: Line 563: private void closeConfirmationWindow() { Line 1059: } Line 1060: Line 1061: public void setStopVolumeProfilingCommand(UICommand stopVolumeProfilingCommand) { Line 1062: this.stopVolumeProfilingCommand = stopVolumeProfilingCommand; Line 1063: } move these setters/getters to the top of this class where other setters/getters are present http://gerrit.ovirt.org/#/c/27470/29/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/VolumeProfileStatisticsPopupView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/VolumeProfileStatisticsPopupView.java: Line 168: /* Line 169: * volumeProfileStats.addEntityModelColumn(new EntityModelTextColumn<FopStats>() { Line 170: * Line 171: * @Override protected String getText(FopStats entity) { return ""; } }, constants.fOpLatency()); Line 172: */ Remove commented code Line 173: Line 174: volumeProfileStats.addEntityModelColumn(new EntityModelTextColumn<FopStats>() { Line 175: @Override Line 176: protected String getText(FopStats entity) { Line 210: /* Line 211: * nfsServerProfileStats.addEntityModelColumn(new EntityModelTextColumn<FopStats>() { Line 212: * Line 213: * @Override protected String getText(FopStats entity) { return ""; } }, constants.fOpLatency()); Line 214: */ same here Line 215: nfsServerProfileStats.addEntityModelColumn(new EntityModelTextColumn<FopStats>() { Line 216: @Override Line 217: protected String getText(FopStats entity) { Line 218: return entity.getMaxLatencyFormatted().getFirst() + " " + entity.getMaxLatencyFormatted().getSecond();//$NON-NLS-1$ Line 244: nfsProfileRunTime.setText(object.getNfsProfileRunTime()); Line 245: bytesRead.setText(object.getBytesRead()); Line 246: nfsBytesRead.setText(object.getNfsBytesRead()); Line 247: bytesWritten.setText(object.getBytesWritten()); Line 248: nfsBytesWritten.setText(object.getNfsBytesWritten()); Why are we setting them manually, doesn't editor takes care of this? Line 249: Line 250: ClickHandler brickTabClickHandler = new ClickHandler() { Line 251: @Override Line 252: public void onClick(ClickEvent event) { http://gerrit.ovirt.org/#/c/27470/29/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/VolumeProfileStatisticsPopupView.ui.xml File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/VolumeProfileStatisticsPopupView.ui.xml: Line 31: <e:EntityModelCellTable ui:field="volumeProfileStats" /> Line 32: </g:ScrollPanel> Line 33: </g:HorizontalPanel> Line 34: <g:FlowPanel> Line 35: <g:HorizontalPanel> Remove this TWS Line 36: <g:Label ui:field="profileRunTime" /> Line 37: </g:HorizontalPanel> Line 38: <g:HorizontalPanel> Line 39: <g:Label ui:field="bytesRead" /> Line 34: <g:FlowPanel> Line 35: <g:HorizontalPanel> Line 36: <g:Label ui:field="profileRunTime" /> Line 37: </g:HorizontalPanel> Line 38: <g:HorizontalPanel> Why do you need a layout panel for one field? Line 39: <g:Label ui:field="bytesRead" /> Line 40: </g:HorizontalPanel> Line 41: <g:HorizontalPanel> Line 42: <g:Label ui:field="bytesWritten" /> Line 63: </g:ScrollPanel> Line 64: </g:HorizontalPanel> Line 65: <g:FlowPanel> Line 66: <g:HorizontalPanel> Line 67: <g:Label ui:field="nfsProfileRunTime" /> Same here. Line 68: </g:HorizontalPanel> Line 69: <g:HorizontalPanel> Line 70: <g:Label ui:field="nfsBytesRead" /> Line 71: </g:HorizontalPanel> -- To view, visit http://gerrit.ovirt.org/27470 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic305a0fece18f29d24a9d0324391e484681fa033 Gerrit-PatchSet: 29 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: anmolbabu <[email protected]> Gerrit-Reviewer: Kanagaraj M <[email protected]> Gerrit-Reviewer: Ramesh N <[email protected]> Gerrit-Reviewer: Sahina Bose <[email protected]> Gerrit-Reviewer: Shubhendu Tripathi <[email protected]> Gerrit-Reviewer: anmolbabu <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
