Freddy Rolland has posted comments on this change. Change subject: webadmin: Add support of refresh LUN size in UI ......................................................................
Patch Set 3: (14 comments) https://gerrit.ovirt.org/#/c/41398/3//COMMIT_MSG Commit Message: Line 9: Add a new column in "Edit Domain" of iSCSI Domain. Line 10: The column is available on ly in "LUNs->Targets" view. Line 11: Line 12: Add a new cell renderer of type toggle button. Line 13: > please add a link to the feature page Done Line 14: Change-Id: I84c83f07f53d286f62fb6104f62fbfb37feb2b42 Line 15: Relates-To: https://bugzilla.redhat.com/609689 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..) Done 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'. It should be "Additional Size" but it was too long. I will try "Size to add" 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 Done 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 CommonApplicat Done 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.. Done 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 Done Line 273: additionalAvailableSize = 0; Line 274: } Line 275: return additionalAvailableSize; Line 276: } Line 650: > move '{' to EOL Done 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 Done 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. Done 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 Done 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 Done 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? No. Same as ExtendSan 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 Done 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
