Daniel Erez has posted comments on this change. Change subject: webadmin: introduce disk profiles ......................................................................
Patch Set 5: (38 comments) http://gerrit.ovirt.org/#/c/29672/5//COMMIT_MSG Commit Message: Line 4: Commit: Gilad Chaplik <gchap...@redhat.com> Line 5: CommitDate: 2014-07-23 21:24:00 +0300 Line 6: Line 7: webadmin: introduce disk profiles Line 8: please elaborate on each item Line 9: * disk profiles subtab under storage main tab. Line 10: * create/update/remove dialog disk profile. Line 11: * backend integration in GUI. Line 12: Line 8: Line 9: * disk profiles subtab under storage main tab. Line 10: * create/update/remove dialog disk profile. Line 11: * backend integration in GUI. Line 12: please consider adding screenshots Line 13: Change-Id: Ibe5cea3c563cf68efca0468749338a532894f709 Line 9: * disk profiles subtab under storage main tab. Line 10: * create/update/remove dialog disk profile. Line 11: * backend integration in GUI. Line 12: Line 13: Change-Id: Ibe5cea3c563cf68efca0468749338a532894f709 add feature page url http://gerrit.ovirt.org/#/c/29672/5/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/profiles/DiskProfileBaseModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/profiles/DiskProfileBaseModel.java: Line 28: import org.ovirt.engine.ui.uicompat.ConstantsManager; Line 29: import org.ovirt.engine.ui.uicompat.FrontendActionAsyncResult; Line 30: import org.ovirt.engine.ui.uicompat.IFrontendActionAsyncCallback; Line 31: Line 32: public abstract class DiskProfileBaseModel extends Model { can u share code with VnicProfileModel? (creating abstract base/etc..) Line 33: private final static StorageQos EMPTY_QOS; Line 34: Line 35: static { Line 36: EMPTY_QOS = new StorageQos(); Line 39: } Line 40: Line 41: private EntityModel<String> name; Line 42: private EntityModel<String> description; Line 43: private final EntityModel sourceModel; use generics if applicable Line 44: private ListModel<StorageDomain> storageDomains; Line 45: private ListModel<StorageQos> qos; Line 46: private DiskProfile diskProfile = null; Line 47: private final Guid defaultQosId; Line 42: private EntityModel<String> description; Line 43: private final EntityModel sourceModel; Line 44: private ListModel<StorageDomain> storageDomains; Line 45: private ListModel<StorageQos> qos; Line 46: private DiskProfile diskProfile = null; why initialize? Line 47: private final Guid defaultQosId; Line 48: private final VdcActionType vdcActionType; Line 49: Line 50: public EntityModel<String> getName() Line 192: public void onSuccess(Object model, Object returnValue) { Line 193: List<StorageQos> qosList = Line 194: returnValue == null ? new ArrayList<StorageQos>() Line 195: : (List<StorageQos>) ((VdcQueryReturnValue) returnValue).getReturnValue(); Line 196: qosList.add(0, EMPTY_QOS); why adding an empty one? Line 197: getQos().setItems(qosList); Line 198: if (defaultQosId != null) { Line 199: for (StorageQos storageQos : qosList) { Line 200: if (defaultQosId.equals(storageQos.getId())) { Line 195: : (List<StorageQos>) ((VdcQueryReturnValue) returnValue).getReturnValue(); Line 196: qosList.add(0, EMPTY_QOS); Line 197: getQos().setItems(qosList); Line 198: if (defaultQosId != null) { Line 199: for (StorageQos storageQos : qosList) { try using 'setItems(Iterable<T> value, T selectedItem)' Line 200: if (defaultQosId.equals(storageQos.getId())) { Line 201: getQos().setSelectedItem(storageQos); Line 202: break; Line 203: } Line 207: })); Line 208: } Line 209: Line 210: public boolean validate() { Line 211: getName().validateEntity(new IValidation[] { new NotEmptyValidation(), new AsciiNameValidation() }); why not using SpecialAsciiI18NOrNoneValidation ? Line 212: Line 213: return getName().getIsValid(); Line 214: } Line 215: http://gerrit.ovirt.org/#/c/29672/5/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/profiles/DiskProfileListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/profiles/DiskProfileListModel.java: Line 25: import org.ovirt.engine.ui.uicompat.ConstantsManager; Line 26: import org.ovirt.engine.ui.uicompat.PropertyChangedEventArgs; Line 27: Line 28: @SuppressWarnings("unused") Line 29: public class DiskProfileListModel extends SearchableListModel can u share code with VnicProfileListModel (abstract/etc..) Line 30: { Line 31: private UICommand newCommand; Line 32: private UICommand editCommand; Line 33: private UICommand removeCommand; Line 70: removeCommand = value; Line 71: } Line 72: Line 73: private Version getCompatibilityVersion() { Line 74: // TODO: please do? :) Line 75: return null; Line 76: } Line 77: Line 78: public void newProfile() { Line 91: if (getWindow() != null) { Line 92: return; Line 93: } Line 94: Line 95: EditDiskProfileModel model = format Line 96: new EditDiskProfileModel(this, Line 97: getCompatibilityVersion(), Line 98: (DiskProfile) getSelectedItem(), Line 99: getEntity().getStoragePoolId()); Line 121: setWindow(null); Line 122: } Line 123: Line 124: @Override Line 125: public StorageDomain getEntity() { please gather all getters/setters in one location Line 126: return (StorageDomain) ((super.getEntity() instanceof StorageDomain) ? super.getEntity() : null); Line 127: } Line 128: Line 129: public void setEntity(StorageDomain value) { Line 165: for (StorageQos storageQos : qosList) { Line 166: qosMap.put(storageQos.getId(), storageQos); Line 167: } Line 168: } Line 169: Frontend.getInstance().runQuery(VdcQueryType.GetDiskProfilesByStorageDomainId, too many canonical queries :) please try to extract properly Line 170: new IdQueryParameters(DiskProfileListModel.this.getEntity().getId()), Line 171: new AsyncQuery(new INewAsyncCallback() { Line 172: Line 173: @Override Line 171: new AsyncQuery(new INewAsyncCallback() { Line 172: Line 173: @Override Line 174: public void onSuccess(Object model1, Object returnValue1) { Line 175: DiskProfileListModel.this.setItems((List<DiskProfile>) ((VdcQueryReturnValue) returnValue1).getReturnValue()); prefix could be removed once propely extracted Line 176: } Line 177: })); Line 178: } Line 179: })); Line 178: } Line 179: })); Line 180: } Line 181: Line 182: public StorageQos getStorageQos(Guid qosId) { please gather all getters/setters in one location Line 183: return qosMap.get(qosId); Line 184: } Line 185: Line 186: @Override Line 179: })); Line 180: } Line 181: Line 182: public StorageQos getStorageQos(Guid qosId) { Line 183: return qosMap.get(qosId); setter? Line 184: } Line 185: Line 186: @Override Line 187: protected void entityPropertyChanged(Object sender, PropertyChangedEventArgs e) { http://gerrit.ovirt.org/#/c/29672/5/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/profiles/RemoveDiskProfileModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/profiles/RemoveDiskProfileModel.java: Line 19: Line 20: public class RemoveDiskProfileModel extends ConfirmationModel { Line 21: Line 22: private final List<DiskProfile> profiles; Line 23: private final boolean fullMsg; needed as class member Line 24: private final ListModel sourceListModel; Line 25: Line 26: public RemoveDiskProfileModel(ListModel sourceListModel, List<DiskProfile> profiles, boolean isFullMsg) { Line 27: setHelpTag(HelpTag.remove_disk_profile); Line 29: setHashName("remove_disk_prfoile"); //$NON-NLS-1$ Line 30: Line 31: this.sourceListModel = sourceListModel; Line 32: this.profiles = profiles; Line 33: this.fullMsg = isFullMsg; what's the usage of this flag? please add comment on this Line 34: Line 35: ArrayList<String> items = new ArrayList<String>(); Line 36: for (DiskProfile profile : profiles) { Line 37: if (isFullMsg) { Line 34: Line 35: ArrayList<String> items = new ArrayList<String>(); Line 36: for (DiskProfile profile : profiles) { Line 37: if (isFullMsg) { Line 38: items.add(getRemoveDiskProfileFullMsg(profile)); why do we need both versions (full/short..)? Line 39: } else { Line 40: items.add(profile.getName()); Line 41: } Line 42: } Line 41: } Line 42: } Line 43: setItems(items); Line 44: Line 45: UICommand tempVar = new UICommand("OnRemove", this); //$NON-NLS-1$ rename Line 46: tempVar.setTitle(ConstantsManager.getInstance().getConstants().ok()); Line 47: tempVar.setIsDefault(true); Line 48: getCommands().add(tempVar); Line 49: UICommand tempVar2 = new UICommand("Cancel", this); //$NON-NLS-1$ Line 45: UICommand tempVar = new UICommand("OnRemove", this); //$NON-NLS-1$ Line 46: tempVar.setTitle(ConstantsManager.getInstance().getConstants().ok()); Line 47: tempVar.setIsDefault(true); Line 48: getCommands().add(tempVar); Line 49: UICommand tempVar2 = new UICommand("Cancel", this); //$NON-NLS-1$ rename Line 50: tempVar2.setTitle(ConstantsManager.getInstance().getConstants().cancel()); Line 51: tempVar2.setIsCancel(true); Line 52: getCommands().add(tempVar2); Line 53: } Line 56: if (getProgress() != null) { Line 57: return; Line 58: } Line 59: Line 60: ArrayList<VdcActionParametersBase> list = new ArrayList<VdcActionParametersBase>(); rename - a bit more specific :) Line 61: for (DiskProfile profile : getProfiles()) { Line 62: VdcActionParametersBase parameters = getRemoveDiskProfileParams(profile); Line 63: list.add(parameters); Line 64: Line 69: Frontend.getInstance().runMultipleAction(VdcActionType.RemoveDiskProfile, list, Line 70: new IFrontendMultipleActionAsyncCallback() { Line 71: @Override Line 72: public void executed(FrontendMultipleActionAsyncResult result) { Line 73: format Line 74: stopProgress(); Line 75: cancel(); Line 76: Line 77: } http://gerrit.ovirt.org/#/c/29672/5/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/StorageListModel.java: Line 443: model.getRetransmissions().setEntity(connection.getNfsRetrans()); Line 444: model.getTimeout().setEntity(connection.getNfsTimeo()); Line 445: model.getMountOptions().setEntity(connection.getMountOptions()); Line 446: for (EntityModel<NfsVersion> item : model.getVersion().getItems()) { Line 447: EntityModel itemModel = item; relevant to this patch? Line 448: boolean noNfsVersion = itemModel.getEntity() == null && connection.getNfsVersion() == null; Line 449: boolean foundNfsVersion = itemModel.getEntity() != null && Line 450: itemModel.getEntity().equals(connection.getNfsVersion()); Line 451: Line 590: StorageModel storageModel = (StorageModel) getWindow(); Line 591: StorageDomain storage = (StorageDomain) getSelectedItem(); Line 592: model.setStorageDomain(storage); Line 593: Line 594: VDS host = storageModel.getHost().getSelectedItem(); ame Line 595: Guid hostId = host != null && isStorageActive ? host.getId() : null; Line 596: Line 597: AsyncDataProvider.getInstance().getLunsByVgId(new AsyncQuery(model, new INewAsyncCallback() { Line 598: @Override Line 655: } Line 656: Line 657: model.startProgress(ConstantsManager.getInstance().getConstants().importingStorageDomainProgress()); Line 658: Line 659: VDS host = model.getHost().getSelectedItem(); same Line 660: Line 661: // Save changes. Line 662: if (model.getSelectedItem() instanceof NfsStorageModel) Line 663: { Line 821: { Line 822: return; Line 823: } Line 824: Line 825: VDS host = model.getHostList().getSelectedItem(); same Line 826: Line 827: RemoveStorageDomainParameters tempVar = new RemoveStorageDomainParameters(storage.getId()); Line 828: tempVar.setVdsId(host.getId()); Line 829: tempVar.setDoFormat(storage.getStorageDomainType().isDataDomain() ? true Line 1223: StorageModel model = (StorageModel) getWindow(); Line 1224: boolean isNew = model.getStorage() == null; Line 1225: storageModel = model.getSelectedItem(); Line 1226: PosixStorageModel posixModel = (PosixStorageModel) storageModel; Line 1227: path = posixModel.getPath().getEntity(); same Line 1228: Line 1229: storageDomain = isNew ? new StorageDomainStatic() : (StorageDomainStatic) Cloner.clone(selectedItem.getStorageStaticData()); Line 1230: storageDomain.setStorageType(isNew ? storageModel.getType() : storageDomain.getStorageType()); Line 1231: storageDomain.setStorageDomainType(isNew ? storageModel.getRole() : storageDomain.getStorageDomainType()); Line 1231: storageDomain.setStorageDomainType(isNew ? storageModel.getRole() : storageDomain.getStorageDomainType()); Line 1232: storageDomain.setStorageName(model.getName().getEntity()); Line 1233: storageDomain.setDescription(model.getDescription().getEntity()); Line 1234: storageDomain.setComment(model.getComment().getEntity()); Line 1235: storageDomain.setStorageFormat(model.getFormat().getSelectedItem()); same Line 1236: Line 1237: if (isNew) { Line 1238: AsyncDataProvider.getInstance().getStorageDomainsByConnection(new AsyncQuery(this, new INewAsyncCallback() { Line 1239: @Override Line 1273: public void saveNewPosixStorage() { Line 1274: Line 1275: StorageModel model = (StorageModel) getWindow(); Line 1276: PosixStorageModel posixModel = (PosixStorageModel) model.getSelectedItem(); Line 1277: VDS host = model.getHost().getSelectedItem(); same Line 1278: hostId = host.getId(); Line 1279: Line 1280: // Create storage connection. Line 1281: StorageServerConnections connection = new StorageServerConnections(); Line 1280: // Create storage connection. Line 1281: StorageServerConnections connection = new StorageServerConnections(); Line 1282: connection.setconnection(path); Line 1283: connection.setstorage_type(posixModel.getType()); Line 1284: connection.setVfsType(posixModel.getVfsType().getEntity()); same for the rest of this file :) Line 1285: connection.setMountOptions(posixModel.getMountOptions().getEntity()); Line 1286: this.connection = connection; Line 1287: Line 1288: ArrayList<VdcActionType> actionTypes = new ArrayList<VdcActionType>(); http://gerrit.ovirt.org/#/c/29672/5/frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIMessages.java File frontend/webadmin/modules/uicompat/src/main/java/org/ovirt/engine/ui/uicompat/UIMessages.java: Line 242: Line 243: @DefaultMessage("VM Interface Profile {0} from Network {1}") Line 244: String vnicProfileFromNetwork(String vnicProfile, String network); Line 245: Line 246: @DefaultMessage("Disk Profile {0} from Storage Domain {1}") suggestion: add 'was' or 'has been' before the verb Line 247: String diskProfileFromStorageDomain(String diskProfile, String storageDomain); Line 248: Line 249: @DefaultMessage("Vnic {0} from Template {1}") Line 250: String vnicFromTemplate(String vnic, String template); http://gerrit.ovirt.org/#/c/29672/5/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationConstants.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/ApplicationConstants.java: Line 3804: Line 3805: @DefaultStringValue("Count") Line 3806: String iopsCountLabelQosPopup(); Line 3807: Line 3808: @DefaultStringValue("QoS") don't we have a generic constant for that already? Line 3809: String diskProfileQosLabel(); Line 3810: Line 3811: @DefaultStringValue("Disk Profiles") Line 3812: String diskProfilesSubTabLabel(); http://gerrit.ovirt.org/#/c/29672/5/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/storage/SubTabStorageDiskProfilePresenter.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/presenter/tab/storage/SubTabStorageDiskProfilePresenter.java: Line 36: Line 37: @TabInfo(container = StorageSubTabPanelPresenter.class) Line 38: static TabData getTabData(ApplicationConstants applicationConstants, Line 39: SearchableDetailModelProvider<DiskProfile, StorageListModel, DiskProfileListModel> modelProvider) { Line 40: return new ModelBoundTabData(applicationConstants.diskProfilesSubTabLabel(), 9, q: is it the right location - i.e. between which tabs is '9'? Line 41: modelProvider); Line 42: } Line 43: Line 44: @Inject http://gerrit.ovirt.org/#/c/29672/5/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/profile/DiskProfilePopupView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/profile/DiskProfilePopupView.java: Line 91: @Override Line 92: public void edit(DiskProfileBaseModel object) { Line 93: driver.edit(object); Line 94: } Line 95: setTabIndexes? (check if needed) Line 96: @Override Line 97: public DiskProfileBaseModel flush() { Line 98: return driver.flush(); Line 99: } http://gerrit.ovirt.org/#/c/29672/5/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/profile/DiskProfilePopupView.ui.xml File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/profile/DiskProfilePopupView.ui.xml: Line 1: <?xml version="1.0" encoding="UTF-8"?> Line 2: <!DOCTYPE ui:UiBinder SYSTEM "http://dl.google.com/gwt/DTD/xhtml.ent";> Line 3: <ui:UiBinder xmlns:ui="urn:ui:com.google.gwt.uibinder" Line 4: xmlns:g="urn:import:com.google.gwt.user.client.ui" tws Line 5: xmlns:d="urn:import:org.ovirt.engine.ui.common.widget.dialog" Line 6: xmlns:e="urn:import:org.ovirt.engine.ui.common.widget.editor" Line 7: xmlns:ge="urn:import:org.ovirt.engine.ui.common.widget.editor.generic" > Line 8: http://gerrit.ovirt.org/#/c/29672/5/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/storage/SubTabStorageDiskProfileView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/storage/SubTabStorageDiskProfileView.java: Line 28: Line 29: void initTable(final ApplicationConstants constants) { Line 30: getTable().enableColumnResizing(); Line 31: Line 32: TextColumnWithTooltip<DiskProfile> nameColumn = sorting Line 33: new TextColumnWithTooltip<DiskProfile>() { Line 34: @Override Line 35: public String getValue(DiskProfile object) { Line 36: return object.getName(); -- To view, visit http://gerrit.ovirt.org/29672 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibe5cea3c563cf68efca0468749338a532894f709 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@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
