Daniel Erez has posted comments on this change. Change subject: webadmin: Add support of refresh LUN size in UI ......................................................................
Patch Set 3: (13 comments) https://gerrit.ovirt.org/#/c/41398/3/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties: Line 808: ACTION_TYPE_FAILED_REFRESH_LUNS_UNSUPPORTED_ACTION better move it to the core patch (as the rest-api can use it..) https://gerrit.ovirt.org/#/c/41398/3/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationConstants.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationConstants.java: Line 1381: . hmm, looks a bit weird.. maybe simply 'Size to ADD'. btw, 'Dev. Size' is just a short of 'Device Size'. https://gerrit.ovirt.org/#/c/41398/3/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/column/AbstractLunAvailableSizeColumn.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/column/AbstractLunAvailableSizeColumn.java: Line 20: @Override Line 21: public void render(Context context, LunModel value, SafeHtmlBuilder sb, String id) { Line 22: boolean isGrayedOut = value.getIsGrayedOut(); Line 23: String inputId = id + "_input"; //$NON-NLS-1$ Line 24: redundant black line Line 25: SafeHtml input = null; Line 26: Line 27: int additionalAvailableSizeSize = value.getAdditionalAvailableSize(); Line 28: String additionalAvailableSizeSizeString = "+ " + //$NON-NLS-1$ Line 24: Line 25: SafeHtml input = null; Line 26: Line 27: int additionalAvailableSizeSize = value.getAdditionalAvailableSize(); Line 28: String additionalAvailableSizeSizeString = "+ " + //$NON-NLS-1$ Instead of manually building the string, create a message in CommonApplicationMessages. for example, see ApplicationMessages - > percentWithValueInGB(). Line 29: additionalAvailableSizeSize + " GB"; //$NON-NLS-1$ Line 30: Line 31: if (additionalAvailableSizeSize == 0 || !value.getIsIncluded()) { Line 32: input = templates.disabled("", "color:gray", inputId); //$NON-NLS-1$ https://gerrit.ovirt.org/#/c/41398/3/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/SanStorageModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/storage/SanStorageModel.java: Line 267: proposeDiscover(); Line 268: } Line 269: Line 270: private int getAdditionalAvailableSize(LUNs lun) { Line 271: int additionalAvailableSize = lun.getDeviceSize() - lun.getPvSize() -1; please add a comment explaining this calculation.. Line 272: if(additionalAvailableSize < 0){ Line 273: additionalAvailableSize = 0; Line 274: } Line 275: return additionalAvailableSize; Line 268: } Line 269: Line 270: private int getAdditionalAvailableSize(LUNs lun) { Line 271: int additionalAvailableSize = lun.getDeviceSize() - lun.getPvSize() -1; Line 272: if(additionalAvailableSize < 0){ formatter Line 273: additionalAvailableSize = 0; Line 274: } Line 275: return additionalAvailableSize; Line 276: } Line 650: move '{' to EOL Line 651: ArrayList<LunModel> luns = new ArrayList<LunModel>(); Line 652: if (!getIsGrouppedByTarget()) { Line 653: List<LunModel> items = (List<LunModel>) getItems(); Line 654: for (LunModel lun : items) Line 655: { same Line 656: if (lun.getIsIncluded() && lun.isAdditionalAvailableSizeSelected() Line 657: && Linq.firstOrDefault(luns, new Linq.LunPredicate(lun)) == null) Line 658: { Line 659: luns.add(lun); Line 653: List<LunModel> items = (List<LunModel>) getItems(); Line 654: for (LunModel lun : items) Line 655: { Line 656: if (lun.getIsIncluded() && lun.isAdditionalAvailableSizeSelected() Line 657: && Linq.firstOrDefault(luns, new Linq.LunPredicate(lun)) == null) a bit hard to read.. consider splitting/extracting these conditions. Line 658: { Line 659: luns.add(lun); Line 660: } Line 661: } Line 654: for (LunModel lun : items) Line 655: { Line 656: if (lun.getIsIncluded() && lun.isAdditionalAvailableSizeSelected() Line 657: && Linq.firstOrDefault(luns, new Linq.LunPredicate(lun)) == null) Line 658: { same Line 659: luns.add(lun); Line 660: } Line 661: } Line 662: } https://gerrit.ovirt.org/#/c/41398/3/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 1597: null, this); Line 1598: } Line 1599: Line 1600: ArrayList<String> lunToRefreshIds = new ArrayList<String>(); Line 1601: redundant blank line Line 1602: for (LunModel lun : sanStorageModel.getLunsToRefresh()) { Line 1603: lunToRefreshIds.add(lun.getLunId()); Line 1604: } Line 1605: Line 1603: lunToRefreshIds.add(lun.getLunId()); Line 1604: } Line 1605: Line 1606: if (lunToRefreshIds.size() > 0) { Line 1607: Frontend.getInstance().runAction(VdcActionType.RefreshLunsSize, is it a synchronous action? Line 1608: new ExtendSANStorageDomainParameters(storageDomain1.getId(), lunToRefreshIds, false), Line 1609: null, this); Line 1610: } Line 1611: Line 1609: null, this); Line 1610: } Line 1611: Line 1612: storageListModel.onFinish(storageListModel.context, true, storageListModel.storageModel); Line 1613: same Line 1614: } Line 1615: }, this); Line 1616: } Line 1617: } -- To view, visit https://gerrit.ovirt.org/41398 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I84c83f07f53d286f62fb6104f62fbfb37feb2b42 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Fred Rolland <froll...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches