Kanagaraj M has posted comments on this change. Change subject: webadmin: UI to set the volume snapshot configurations ......................................................................
Patch Set 10: (11 comments) http://gerrit.ovirt.org/#/c/36295/10/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterClusterSnapshotConfigModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterClusterSnapshotConfigModel.java: Line 74: } Line 75: Line 76: private void setAvailabilities() { Line 77: getClusters().setIsAvailable(true); Line 78: getClusterConfigOptions().setIsAvailable(true); Setting these explicitly is not required since they default to true Line 79: } Line 80: Line 81: public boolean validate() { Line 82: boolean isValid = true; Line 127: // set the entity items Line 128: listModel.setItems(coll); Line 129: } Line 130: }), Line 131: getClusters().getSelectedItem().getId(), possible NPE if default cluster is not set Line 132: null); Line 133: } http://gerrit.ovirt.org/#/c/36295/10/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterVolumeSnapshotConfigModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/GlusterVolumeSnapshotConfigModel.java: Line 98: Line 99: private void setAvailabilities() { Line 100: getClusterName().setIsAvailable(true); Line 101: getVolumeName().setIsAvailable(true); Line 102: getConfigOptions().setIsAvailable(true); same here Line 103: } Line 104: Line 105: public boolean validate() { Line 106: boolean isValid = true; Line 118: Line 119: return isValid; Line 120: } Line 121: Line 122: private void populateConfigOptions() { Use startProgress and stopProgress to avoid displaying an empty dialog Line 123: AsyncDataProvider.getInstance().getGlusterSnapshotConfig(new AsyncQuery(this, new INewAsyncCallback() { Line 124: Line 125: @Override Line 126: public void onSuccess(Object model, Object returnValue) { Line 136: EntityModel<VolumeSnapshotOptionModel> cfgModel = new EntityModel<VolumeSnapshotOptionModel>(); Line 137: VolumeSnapshotOptionModel option = new VolumeSnapshotOptionModel(); Line 138: option.setOptionName(cfg.getParamName()); Line 139: option.setOptionValue(cfg.getParamValue()); Line 140: for (GlusterVolumeSnapshotConfig clusterCfg : clusterConfigOptions) { you could form a map of Option->Value, so that looping every time can be avoided Line 141: if (clusterCfg.getParamName().equals(cfg.getParamName())) { Line 142: option.setCorrespodingClusterValue(clusterCfg.getParamValue()); Line 143: break; Line 144: } http://gerrit.ovirt.org/#/c/36295/10/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 502: : asyncTask.getStatus() != JobExecutionStatus.STARTED; Line 503: } Line 504: } Line 505: Line 506: if (list.size() == 1) { you could do allowConfigureVolumeSnapshotOptions = volumeEntity.getStatus() == GlusterStatus.UP here itself Line 507: GlusterVolumeEntity volumeEntity = list.get(0); Line 508: GlusterAsyncTask asyncTask = volumeEntity.getAsyncTask(); Line 509: allowStopRebalance = Line 510: volumeEntity.getStatus() == GlusterStatus.UP && asyncTask != null Line 1263: GlusterClusterSnapshotConfigModel clusterSnapshotConfigModel = (GlusterClusterSnapshotConfigModel) getWindow(); Line 1264: Line 1265: if (!clusterSnapshotConfigModel.validate()) { Line 1266: return; Line 1267: } validation should be done in the above function itself Line 1268: Line 1269: Guid clusterId = clusterSnapshotConfigModel.getClusters().getSelectedItem().getId(); Line 1270: List<GlusterVolumeSnapshotConfig> vdsParams = new ArrayList<GlusterVolumeSnapshotConfig>(); Line 1271: for (EntityModel<GlusterVolumeSnapshotConfig> clusterCfg : clusterSnapshotConfigModel.getClusterConfigOptions() Line 1368: Line 1369: public void onConfigureVolumeSnapshotOptions() { Line 1370: GlusterVolumeSnapshotConfigModel volumeSnapshotConfigModel = (GlusterVolumeSnapshotConfigModel) getWindow(); Line 1371: Line 1372: if (!volumeSnapshotConfigModel.validate()) { same here Line 1373: return; Line 1374: } Line 1375: Line 1376: GlusterVolumeEntity volumeEntity = volumeSnapshotConfigModel.getSelectedVolumeEntity(); http://gerrit.ovirt.org/#/c/36295/10/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java File frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIConstants.java: Line 2516: Line 2517: @DefaultStringValue("Configuring volume snapshot options\n\n" Line 2518: + "Changing configuration parameters will lead to deletion of snapshots if they exceed the new limit.\n\n" Line 2519: + "Are you sure you want to continue?") Line 2520: String youAreAboutChangeSnapshotConfigurationMsg(); This message might be misleading if the user has modified some configuration which is not snap-max-limit. http://gerrit.ovirt.org/#/c/36295/10/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/GlusterClusterSnapshotConfigureOptionsPopupView.ui.xml File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/GlusterClusterSnapshotConfigureOptionsPopupView.ui.xml: Line 22: padding-bottom: 5px; Line 23: padding-top: 30px; Line 24: } Line 25: Line 26: .messageLabel { is this being used? Line 27: color: #FF0000; Line 28: left: 10px; Line 29: padding-left: 5px; Line 30: } http://gerrit.ovirt.org/#/c/36295/10/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/GlusterVolumeSnapshotConfigureOptionsPopupView.ui.xml File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/GlusterVolumeSnapshotConfigureOptionsPopupView.ui.xml: Line 22: padding-bottom: 5px; Line 23: padding-top: 30px; Line 24: } Line 25: Line 26: .messageLabel { same here Line 27: color: #FF0000; Line 28: left: 10px; Line 29: padding-left: 5px; Line 30: } -- To view, visit http://gerrit.ovirt.org/36295 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I08b5e592b818266106bc1891c3aa2f3ba3541d36 Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Shubhendu Tripathi <shtri...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Shubhendu Tripathi <shtri...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: anmolbabu <anb...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
