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

Reply via email to