Yevgeny Zaspitsky has posted comments on this change. Change subject: webadmin: Update HostSetupNetworks flow to use isManagement ......................................................................
Patch Set 3: (6 comments) http://gerrit.ovirt.org/#/c/37028/3/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Linq.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/Linq.java: Line 492: } Line 493: return ret; Line 494: } Line 495: Line 496: public static Network findManagentNetwork(List<Network> networks) { > Typo. Done Line 497: for (Network network : networks) { Line 498: if (isManagementNetwork(network)) { Line 499: return network; Line 500: } 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 ini Done 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 metho Done 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 referen Done 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 Better safe than sorry 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 cou the member is used not only for caching. setManagement method is used from other classes. 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: Yevgeny Zaspitsky <yzasp...@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
