Lior Vernia has posted comments on this change. Change subject: webadmin: Update HostSetupNetworks flow to use isManagement ......................................................................
Patch Set 3: (5 comments) http://gerrit.ovirt.org/#/c/37028/3/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostSetupNetworksModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/HostSetupNetworksModel.java: Line 66: public class HostSetupNetworksModel extends EntityModel<VDS> { Line 67: Line 68: private EntityModel<Boolean> checkConnectivity; Line 69: Line 70: private Network managentNetwork; Typo. Also, consider saving a reference to the LogicalNetworkModel from initNetworkModels - this will render some other code unnecessary. Line 71: Line 72: public EntityModel<Boolean> getCheckConnectivity() Line 73: { Line 74: return checkConnectivity; Line 882: public void onSuccess(Object model, Object returnValue) Line 883: { Line 884: List<Network> networks = (List<Network>) returnValue; Line 885: allNetworks = networks; Line 886: managentNetwork = Linq.findManagentNetwork(allNetworks); This will be replaced by a check in initNetworkModels(), and the Linq method can be removed as well. Line 887: initNetworkModels(); Line 888: initDcNetworkParams(); Line 889: Line 890: // chain the nic query Line 927: } Line 928: Line 929: private void validate() { Line 930: // check if management network is attached Line 931: final LogicalNetworkModel mgmtNetwork = networkMap.get(managentNetwork.getName()); And here you won't need to fetch from a map - you'll already have a reference to the correct model. Line 932: if (!mgmtNetwork.isAttached()) { Line 933: okCommand.getExecuteProhibitionReasons().add(ConstantsManager.getInstance() Line 934: .getConstants() Line 935: .mgmtNotAttachedToolTip()); http://gerrit.ovirt.org/#/c/37028/3/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/LogicalNetworkModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/hosts/network/LogicalNetworkModel.java: Line 29: Line 30: public LogicalNetworkModel(Network network, HostSetupNetworksModel setupModel) { Line 31: this(setupModel); Line 32: setEntity(network); Line 33: if (network.getCluster() != null && network.getCluster().isManagement()) { The null check is overly-protective - in this context you should be able to make sure all networks have the field set (otherwise you're in trouble, as it's possible the management network itself should have this not set). Line 34: setManagement(true); Line 35: } Line 36: } Line 37: Line 30: public LogicalNetworkModel(Network network, HostSetupNetworksModel setupModel) { Line 31: this(setupModel); Line 32: setEntity(network); Line 33: if (network.getCluster() != null && network.getCluster().isManagement()) { Line 34: setManagement(true); The management network member is no longer needed, it's redundant - you could have isManagementNetwork() return getEntity().getCluster().isManagement() instead. Line 35: } Line 36: } Line 37: Line 38: /** -- To view, visit http://gerrit.ovirt.org/37028 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3bb6d8149d895a5834c5fb59e357a9acab7cb00d Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yevgeny Zaspitsky <yzasp...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@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
