Eldan Shachar 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 Done 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 wa Created a TypeAhedChangeableListBox and integrated it as a replacement for the combobox. 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 Used the new listBox. 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> Done 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? Replaced it with constants from the backend. the limits are based on a reasonable use of the fields, the current supported values are significantly smaller but they may be larger in the future. (the actual limit is in the db of course) 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 = ... Done 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 t Done 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<... Done 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 Done 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$ the minus is unescaped so it must be the last char in the list. added the plus sign before the super. 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: 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
