Ramesh N has posted comments on this change. Change subject: gluster: Add support for gluster brick creation ......................................................................
Patch Set 13: (12 comments) https://gerrit.ovirt.org/#/c/36031/13/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/HostStorageDevicesListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/HostStorageDevicesListModel.java: Line 32: setTitle(ConstantsManager.getInstance().getConstants().storageDevices()); Line 33: setHelpTag(HelpTag.storage_device_list); Line 34: setHashName("storage_device_list"); //$NON-NLS-1$ Line 35: setCreateBrickCommand(new UICommand("createBrick", this)); //$NON-NLS-1$ Line 36: setSyncStorageDevicesCommand(new UICommand("Sync", this)); //$NON-NLS-1$ > Please undo the above change Done Line 37: setAvailableInModes(ApplicationMode.GlusterOnly); Line 38: } Line 39: Line 40: @Override Line 119: return; Line 120: } Line 121: Line 122: VDS host = getEntity(); Line 123: if (host == null || host.getStatus() != VDSStatus.Up) { > If this is the case, can we either open the dialog with a warning or even b Disabled the command. Line 124: return; Line 125: } Line 126: Line 127: final LogicalVolumeModel lvModel = new LogicalVolumeModel(); Line 137: Line 138: @Override Line 139: public void onSuccess(Object model, Object returnValue) { Line 140: lvModel.stopProgress(); Line 141: lvModel.initSize(); > Any reason why this is not invoked from with constructor of lvModel? Done Line 142: LogicalVolumeModel lvModel = (LogicalVolumeModel) model; Line 143: List<StorageDevice> devices = (List<StorageDevice>) returnValue; Line 144: Line 145: for (StorageDevice device : devices) { Line 171: .getConfigFromCache(new GetConfigurationValueParameters(ConfigurationValues.DefaultGlusterBrickMountPoint, Line 172: AsyncDataProvider.getInstance().getDefaultConfigurationVersion()), Line 173: asyncQueryForDefaultMountPoint); Line 174: Line 175: UICommand okCommand = new UICommand("OnCreateBrick", this); //$NON-NLS-1$ > UICommand.createDefaultOkUiCommand ? Done Line 176: okCommand.setTitle(ConstantsManager.getInstance().getConstants().ok()); Line 177: okCommand.setIsDefault(true); Line 178: lvModel.getCommands().add(okCommand); Line 179: Line 176: okCommand.setTitle(ConstantsManager.getInstance().getConstants().ok()); Line 177: okCommand.setIsDefault(true); Line 178: lvModel.getCommands().add(okCommand); Line 179: Line 180: UICommand cancelCommand = new UICommand("Cancel", this); //$NON-NLS-1$ > UICommand.createCancelUiCommand ? Done Line 181: cancelCommand.setTitle(ConstantsManager.getInstance().getConstants().cancel()); Line 182: cancelCommand.setIsCancel(true); Line 183: lvModel.getCommands().add(cancelCommand); Line 184: } Line 200: if (host == null) { Line 201: return; Line 202: } Line 203: Line 204: setConfirmWindow(null); > Importance of the above setConfirmWindow(null);? NOt required. Removed. Line 205: lvModel.startProgress(null); Line 206: Line 207: List<StorageDevice> selectedDevices = new ArrayList<StorageDevice>(); Line 208: for (StorageDevice device : lvModel.getStorageDevices().getSelectedItems()) { Line 229: Line 230: private void postCreateBrick() { Line 231: LogicalVolumeModel model = (LogicalVolumeModel) getWindow(); Line 232: model.stopProgress(); Line 233: setWindow(null); > what if the above action fails? Please check if this closes the error dialo will the callback.executed() will be called even in case of failure? Line 234: } Line 235: Line 236: @Override Line 237: public void executeCommand(UICommand command) { https://gerrit.ovirt.org/#/c/36031/13/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/LogicalVolumeModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/gluster/LogicalVolumeModel.java: Line 95: getSize().setEntity(sizeString); Line 96: } Line 97: Line 98: private String getSizeString(Pair<SizeUnit, Double> size) { Line 99: return formatSize(size.getSecond()) + " " + size.getFirst().toString();//$NON-NLS-1$ > May be this needs to be localised? UIMessages? Lets keep it as it is for now. Line 100: } Line 101: Line 102: public ListModel<StorageDevice> getStorageDevices() { Line 103: return storageDevices; https://gerrit.ovirt.org/#/c/36031/13/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/CreateBrickPopupView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/CreateBrickPopupView.java: Line 139: deviceTable.addColumnAndSetWidth(new TextColumnWithTooltip<StorageDevice>() { Line 140: @Override Line 141: public String getValue(StorageDevice entity) { Line 142: Pair<SizeUnit, Double> convertedSize = SizeConverter.autoConvert(entity.getSize(), SizeUnit.MB); Line 143: return formatSize(convertedSize.getSecond()) + " " + convertedSize.getFirst().toString(); //$NON-NLS-1$ > Concern of localisation as before lets keep it as it is. Line 144: } Line 145: }, constants.size(), "100px"); //$NON-NLS-1$ Line 146: deviceSelectionInfo.setVisible(false); Line 147: } Line 165: Line 166: @Override Line 167: public void eventRaised(Event<? extends EventArgs> ev, Object sender, EventArgs args) { Line 168: if (!object.getRaidTypeList().getSelectedItem().equals(RaidType.None) Line 169: && !object.getRaidTypeList().getSelectedItem().equals(RaidType.Raid0)) { > Please move this to presenterwidget as it involves view-model interaction Moving it to model Line 170: NoOfPhysicalDisksEditor.setVisible(true); Line 171: stripeSizeEditor.setVisible(true); Line 172: deviceSelectionInfo.setVisible(true); Line 173: deviceSelectionInfo.setText(constants.getStorageDeviceSelectionInfo() https://gerrit.ovirt.org/#/c/36031/13/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/CreateBrickPopupView.ui.xml File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/gluster/CreateBrickPopupView.ui.xml: Line 22: } Line 23: .explanationLabel { Line 24: font-style: italic; Line 25: padding-bottom: 10px; Line 26: } > TWS Done Line 27: </ui:style> Line 28: Line 29: <d:SimpleDialogPanel width="550px" height="575px"> Line 30: <d:content> Line 40: <g:VerticalPanel> Line 41: <g:ScrollPanel addStyleNames="{style.tablePanel}"> Line 42: <e:ListModelObjectCellTable ui:field="deviceTable"/> Line 43: </g:ScrollPanel> Line 44: </g:VerticalPanel> > TWS Done Line 45: </g:HorizontalPanel> Line 46: <ge:StringEntityModelLabelEditor ui:field="sizeEditor" /> Line 47: </g:FlowPanel> Line 48: </d:content> -- To view, visit https://gerrit.ovirt.org/36031 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If43b6503dd8362d2a0907ac9648335a750828427 Gerrit-PatchSet: 13 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ramesh N <rnach...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Ramesh N <rnach...@redhat.com> Gerrit-Reviewer: Sahina Bose <sab...@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
