anmolbabu has posted comments on this change. Change subject: webadmin : Gluster Volume Profile ......................................................................
Patch Set 34: (8 comments) http://gerrit.ovirt.org/#/c/27470/34/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeProfileStatisticsModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/VolumeProfileStatisticsModel.java: Line 46: Line 47: private VolumeListModel volumeListModel; Line 48: private GlusterVolumeProfileInfo profileInfo; Line 49: Line 50: private static ProfileTabSelected currentTab; > Avoid using static fields unless its really required Done Line 51: Line 52: public static enum ProfileTabSelected { Line 53: BRICKS(0) , Line 54: NFS(1); Line 48: private GlusterVolumeProfileInfo profileInfo; Line 49: Line 50: private static ProfileTabSelected currentTab; Line 51: Line 52: public static enum ProfileTabSelected { > Do we need this? Done Line 53: BRICKS(0) , Line 54: NFS(1); Line 55: Line 56: private int weight; Line 173: stopProgress(); Line 174: VdcQueryReturnValue vdcValue = (VdcQueryReturnValue) returnValue; Line 175: GlusterVolumeProfileInfo profileInfoEntity =vdcValue.getReturnValue(); Line 176: if((profileInfoEntity == null) || (!vdcValue.getSucceeded())) { Line 177: volumeListModel.showProfileErrorWindow(); > is this required? yes bcoz,the last time I tried getWindow was for some reasin returning null here instead of returning VolumeProfileStatisticsModel.Not sure if this is bcoz this model is itself the window(popup) of interest.And we need to somehow stop the progress on this model.Or may be I should change this to stopprogress simply. Line 178: onPropertyChanged(new PropertyChangedEventArgs("errorFetchingProfileStats"));//$NON-NLS-1$ Line 179: if(currentTab == ProfileTabSelected.NFS) { Line 180: showNfsProfileStats(profileInfoEntity); Line 181: } else { http://gerrit.ovirt.org/#/c/27470/34/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 772: private void showVolumeProfiling() { Line 773: if(getSelectedItem() == null) { Line 774: return; Line 775: } Line 776: VolumeProfileStatisticsModel profileStatsModel = new VolumeProfileStatisticsModel(((GlusterVolumeEntity)getSelectedItem()).getClusterId(), ((GlusterVolumeEntity)getSelectedItem()).getId(), ((GlusterVolumeEntity)getSelectedItem()).getName(), this); > (GlusterVolumeEntity)getSelectedItem() can be assigned to a local fields, s Done Line 777: Line 778: VolumeProfileStatisticsModel.setCurrentTab(VolumeProfileStatisticsModel.ProfileTabSelected.BRICKS); Line 779: setProfileWindow(profileStatsModel); Line 780: profileStatsModel.startProgress(ConstantsManager.getInstance().getConstants().fetchingDataMessage());//$NON-NLS-1$ Line 786: Line 787: profileStatsModel.queryBackend(); Line 788: } Line 789: Line 790: public void showProfileErrorWindow() { > is this being used? Done Line 791: if (getWindow() != null) { Line 792: getWindow().stopProgress(); Line 793: } Line 794: } http://gerrit.ovirt.org/#/c/27470/34/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 118: Line 119: @UiField Line 120: @Ignore Line 121: @WithElementId Line 122: Label nfsBytesWritten; > Can these fields ordered in the way they are displaying in the ui? Done Line 123: Line 124: private final Driver driver = GWT.create(Driver.class); Line 125: Line 126: @Inject Line 253: ClickHandler brickTabClickHandler = new ClickHandler() { Line 254: @Override Line 255: public void onClick(ClickEvent event) { Line 256: VolumeProfileStatisticsModel.setCurrentTab(VolumeProfileStatisticsModel.ProfileTabSelected.BRICKS); Line 257: object.queryBackend(); > you can pass an argument to decide what data to fetch (just boolean would d Done Line 258: } Line 259: }; Line 260: Line 261: brickRefreshIcon.setRefreshIconClickListener(brickTabClickHandler); Line 263: ClickHandler nfsTabClickHandler = new ClickHandler() { Line 264: @Override Line 265: public void onClick(ClickEvent event) { Line 266: VolumeProfileStatisticsModel.setCurrentTab(VolumeProfileStatisticsModel.ProfileTabSelected.NFS); Line 267: object.queryBackend(); > Same here. you could say false to the bool Done Line 268: } Line 269: }; Line 270: Line 271: nfsRefreshIcon.setRefreshIconClickListener(nfsTabClickHandler); -- 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: 34 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
