anmolbabu has posted comments on this change. Change subject: gluster: Add support for gluster brick creation ......................................................................
Patch Set 13: (14 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 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 better disable the command that delegates to here? Bcoz the user clicks the command but sees no dialog? 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? 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 ? 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 ? 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);? 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 dialog as well 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? KiloBytes in japanese is キロバイト May not be used in general but just found that on searching for it :D 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 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 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 Line 27: </ui:style> Line 28: Line 29: <d:SimpleDialogPanel width="550px" height="575px"> Line 30: <d:content> Line 35: <ge:IntegerEntityModelTextBoxEditor ui:field="NoOfPhysicalDisksEditor" /> Line 36: <ge:IntegerEntityModelTextBoxEditor ui:field="stripeSizeEditor" /> Line 37: <g:Label ui:field="deviceHeader" addStyleNames="{style.headerLabel}"/> Line 38: <g:Label ui:field="deviceSelectionInfo" addStyleNames="{style.explanationLabel}"/> Line 39: <g:HorizontalPanel spacing="5" verticalAlignment="ALIGN_MIDDLE"> why is this required? HorizontalPanel Line 40: <g:VerticalPanel> Line 41: <g:ScrollPanel addStyleNames="{style.tablePanel}"> Line 42: <e:ListModelObjectCellTable ui:field="deviceTable"/> Line 43: </g:ScrollPanel> Line 36: <ge:IntegerEntityModelTextBoxEditor ui:field="stripeSizeEditor" /> Line 37: <g:Label ui:field="deviceHeader" addStyleNames="{style.headerLabel}"/> Line 38: <g:Label ui:field="deviceSelectionInfo" addStyleNames="{style.explanationLabel}"/> Line 39: <g:HorizontalPanel spacing="5" verticalAlignment="ALIGN_MIDDLE"> Line 40: <g:VerticalPanel> why is this required VerticalPanel? Line 41: <g:ScrollPanel addStyleNames="{style.tablePanel}"> Line 42: <e:ListModelObjectCellTable ui:field="deviceTable"/> Line 43: </g:ScrollPanel> Line 44: </g:VerticalPanel> 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 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 <[email protected]> Gerrit-Reviewer: Kanagaraj M <[email protected]> Gerrit-Reviewer: Sahina Bose <[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
