Tomas Jelinek has posted comments on this change. Change subject: userportal\webadmin: cluster parameters override ......................................................................
Patch Set 6: (10 comments) http://gerrit.ovirt.org/#/c/31582/6//COMMIT_MSG Commit Message: Line 5: CommitDate: 2014-09-28 13:17:13 +0300 Line 6: Line 7: userportal\webadmin: cluster parameters override Line 8: Line 9: This feature allows to configure the 'emulated machine' and 'cpu model'parameters for each VM separately instead of relying on the cluster default. please split this line to 80 chars long lines Line 10: Line 11: Change-Id: Ia288e46f38c03935ec9b0817833b1921f15ec9f8 Line 12: Bug-Url: https://bugzilla.redhat.com/838487 Line 13: Feature-page:http://www.ovirt.org/Features/Cluster_parameters_override http://gerrit.ovirt.org/#/c/31582/6/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/AbstractVmPopupWidget.ui.xml File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/AbstractVmPopupWidget.ui.xml: Line 513: <g:FlowPanel ui:field="vcpusAdvancedParameterExpanderContent" addStyleNames="{style.generalExpanderContent}"> Line 514: <ge:EntityModelDetachableWidgetWithLabel ui:field="corePerSocketEditorWithDetachable" /> Line 515: <ge:EntityModelDetachableWidgetWithLabel ui:field="numOfSocketsEditorWithDetachable" /> Line 516: <w:ComboBox ui:field="emulatedMachineComboBox" /> Line 517: <w:ComboBox ui:field="customCpuComboBox" /> Please use the ListModelSuggestBox instead of the ComboBox widget. It is way better because it integrates properly with the UiCommon, no need to maintain 2 different widgets (list box and text box), looks better and also supports the type ahead together with enabling of adding the free text. Line 518: </g:FlowPanel> Line 519: </g:FlowPanel> Line 520: <g:Label ui:field="generalLabel" addStyleNames="{style.sectionLabel}" text="{constants.initialRunGeneral}" /> Line 521: <w:EntityModelWidgetWithInfo ui:field="timeZoneEditorWithInfo" /> http://gerrit.ovirt.org/#/c/31582/6/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmRunOncePopupWidget.ui.xml File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmRunOncePopupWidget.ui.xml: Line 227: Line 228: <g:DisclosurePanel ui:field="systemPanel" label="{constants.systemVmPopup}" addStyleNames="{style.panelStyle}"> Line 229: <g:FlowPanel> Line 230: <w:ComboBox ui:field="emulatedMachineComboBox" /> Line 231: <w:ComboBox ui:field="customCpuComboBox" /> also here please use the ListModelSuggestBox - it will simplify everything a lot. Line 232: </g:FlowPanel> Line 233: </g:DisclosurePanel> Line 234: Line 235: <g:DisclosurePanel ui:field="hostPanel" label="{constants.hostVmPopup}" addStyleNames="{style.panelStyle}"> http://gerrit.ovirt.org/#/c/31582/6/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/dataprovider/AsyncDataProvider.java: Line 3834: { Line 3835: if (source != null) Line 3836: { Line 3837: ArrayList<VDS> vdsList = Linq.<VDS> cast((ArrayList<IVdcQueryable>) source); Line 3838: HashSet<String> emulatedMachineList = new HashSet(); please change this to Set<String> emulatedMachineList = new HashSet<String>(); Line 3839: for (VDS host : vdsList) { Line 3840: String hostSupportedMachines = host.getSupportedEmulatedMachines(); Line 3841: if(!StringHelper.isNullOrEmpty(hostSupportedMachines)) { Line 3842: emulatedMachineList.addAll(Arrays.asList(hostSupportedMachines.split(","))); //$NON-NLS-1$ http://gerrit.ovirt.org/#/c/31582/6/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/RunOnceModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/RunOnceModel.java: Line 48: Line 49: public abstract class RunOnceModel extends Model Line 50: { Line 51: public static final int EMULATED_MACHINE_MAX_LIMIT = 30; Line 52: public static final int CPU_NAME_MAX_LIMIT = 30; Do we want this constants hard coded here? Where do this limits come from? Don't they depend on something? Line 53: Line 54: // Boot Options tab Line 55: Line 56: public static final String RUN_ONCE_COMMAND = "OnRunOnce"; //$NON-NLS-1$ Line 1025: new INewAsyncCallback() { Line 1026: @Override Line 1027: public void onSuccess(Object model, Object returnValue) { Line 1028: if(returnValue != null) { Line 1029: TreeSet<String> emulatedSet = new TreeSet<String>((HashSet<String>) returnValue); Set<String> emulatedSet = ... Line 1030: emulatedSet.add(""); Line 1031: getEmulatedMachine().setItems(emulatedSet); Line 1032: } Line 1033: } http://gerrit.ovirt.org/#/c/31582/6/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java: Line 76: Line 77: public static final int VM_TEMPLATE_NAME_MAX_LIMIT = 40; Line 78: public static final int DESCRIPTION_MAX_LIMIT = 255; Line 79: public static final int EMULATED_MACHINE_MAX_LIMIT = 30; Line 80: public static final int CPU_NAME_MAX_LIMIT = 30; same. BTW if they really can be hardcoded, please at least don't hardcode them on 2 places. Line 81: Line 82: private boolean privateIsNew; Line 83: Line 84: private EntityModel<Boolean> valid; http://gerrit.ovirt.org/#/c/31582/6/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmModelBehaviorBase.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmModelBehaviorBase.java: Line 1099: new INewAsyncCallback() { Line 1100: @Override Line 1101: public void onSuccess(Object model, Object returnValue) { Line 1102: if(returnValue != null) { Line 1103: TreeSet<String> emulatedSet = new TreeSet<String>((HashSet<String>) returnValue); Set<String> instead of TreeSet<... Line 1104: emulatedSet.add(""); Line 1105: getModel().getEmulatedMachine().setItems(emulatedSet); Line 1106: } Line 1107: } Line 1109: } Line 1110: Line 1111: /* Line 1112: * Updates the cpu model combobox after a cluster change occurs Line 1113: */ formatter Line 1114: protected void updateCustomCpu() { Line 1115: VDSGroup cluster = getModel().getSelectedCluster(); Line 1116: Line 1117: if (cluster == null || cluster.getcpu_name() == null) { http://gerrit.ovirt.org/#/c/31582/6/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/I18NExtraNameOrNoneValidation.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/validation/I18NExtraNameOrNoneValidation.java: Line 11: public class I18NExtraNameOrNoneValidation extends I18NNameValidation { Line 12: Line 13: @Override Line 14: protected String specialCharacters() { Line 15: return "._\\+-"; //$NON-NLS-1$ I'd use super.specialCharacters() + "\\+"; //$NON-NLS-1$ Line 16: } Line 17: Line 18: @Override Line 19: protected String end() { -- To view, visit http://gerrit.ovirt.org/31582 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia288e46f38c03935ec9b0817833b1921f15ec9f8 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eldan Shachar <eshac...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@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
