Tomas Jelinek has posted comments on this change.

Change subject: webadmin, userportal: logical network editor
......................................................................


Patch Set 8: (13 inline comments)

....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ExistingVmModelBehavior.java
Line 245:                 List<VmNetworkInterface> nics = 
(List<VmNetworkInterface>) result;
Line 246:                 initNetworkInterfaces(networkBehavior, nics);
Line 247:             }
Line 248:         };
Line 249:         AsyncDataProvider.getVmNicList(getVmNicsQuery, vm.getId());
Done
Line 250:     }
Line 251: 
Line 252:     @Override
Line 253:     protected void changeDefualtHost()


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NetworkBehavior.java
Line 19: 
Line 20:         query.converterCallback = new IAsyncConverter() {
Line 21: 
Line 22:             @Override
Line 23:             public Object Convert(Object returnValue, AsyncQuery 
asyncQuery) {
:)
Line 24:                 ArrayList<Network> networks = new ArrayList<Network>();
Line 25:                 for (Network a : (ArrayList<Network>) returnValue) {
Line 26:                     if (a.isVmNetwork()) {
Line 27:                         networks.add(a);


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NetworkFrontendActionAsyncCallbacks.java
Line 5: import org.ovirt.engine.core.compat.Guid;
Line 6: import org.ovirt.engine.ui.uicompat.FrontendActionAsyncResult;
Line 7: import org.ovirt.engine.ui.uicompat.IFrontendActionAsyncCallback;
Line 8: 
Line 9: public class NetworkFrontendActionAsyncCallbacks {
Renamed to UnitVmModelNetworkAsyncCallbacks. 

Well, I don't particularly like the idea of making the VmNetworkCreatingManager 
implement IFrontendActionAsyncCallback and doing some if-else according to the 
action type. That is not a responsibility of the manager - the manager is 
generic.

Also I'm not really a fan of having files per that small classes. This classes 
are here only to avoid the boilerplate of handling the results everywhere.

To make the code more readable I have at least imported this classes statically.

But I see your point - this is maybe more a question of personal taste...
Line 10: 
Line 11:     public static abstract class 
BaseNetworkFrontendActionAsyncCallback implements IFrontendActionAsyncCallback {
Line 12: 
Line 13:         private final UnitVmModel unitVmModel;


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewVmModelBehavior.java
Line 199:                 }
Line 200: 
Line 201:                 List<VmNetworkInterface> nics =
Line 202:                         (List<VmNetworkInterface>) 
((VdcQueryReturnValue) returnValue).getReturnValue();
Line 203:                 if (nics.size() == 0) {
Done
Line 204:                     return;
Line 205:                 }
Line 206: 
Line 207:                 initNetworkInterfaces(networkBehavior, nics);


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NullNetwork.java
Line 1: package org.ovirt.engine.ui.uicommonweb.models.vms;
Line 2: 
Line 3: import org.ovirt.engine.core.common.businessentities.network.Network;
Line 4: 
Line 5: public class NullNetwork extends Network {
ok, removed and adjusted the rest of the code to use the null.
Line 6: 
Line 7:     private static final long serialVersionUID = 1L;
Line 8: 
Line 9:     @Override


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/UnitVmModel.java
Line 103:     private void 
setDataCenterWithClustersList(NotChangableForVmInPoolListModel 
dataCenterWithClustersList) {
Line 104:         this.dataCenterWithClustersList = dataCenterWithClustersList;
Line 105:     }
Line 106: 
Line 107:     private ListModel nicWithLogicalNetworks;
Done
Line 108: 
Line 109:     public ListModel getNicWithLogicalNetworks() {
Line 110:         return nicWithLogicalNetworks;
Line 111:     }


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmListModel.java
Line 2009:                 parameters.setSoundDeviceEnabled((Boolean) 
model.getIsSoundcardEnabled().getEntity());
Line 2010: 
Line 2011:                 setVmWatchdogToParams(model, parameters);
Line 2012:                 Frontend.RunAction(VdcActionType.AddVmFromScratch, 
parameters,
Line 2013:                         new IFrontendActionAsyncCallback() {
done - adjusted the networkCreated() so now I could reuse it.
Line 2014:                             @Override
Line 2015:                             public void 
executed(FrontendActionAsyncResult result) {
Line 2016:                                 VdcReturnValueBase returnValueBase = 
result.getReturnValue();
Line 2017:                                 if (returnValueBase != null && 
returnValueBase.getSucceeded()) {


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmModelBehaviorBase.java
Line 907:             @Override
Line 908:             public void onSuccess(Object model, Object returnValue) {
Line 909:                 
getModel().getIsSoundcardEnabled().setEntity(returnValue);
Line 910:             }
Line 911:         }, getModel().getHash()), id);
merging mistake - removed
Line 912:     }
Line 913: 
Line 914:     protected void initNetworkInterfaces(final NetworkBehavior 
behavior, final List<VmNetworkInterface> nics) {
Line 915: 


Line 940:                 
getModel().getNicWithLogicalNetworks().setSelectedItem(Linq.firstOrDefault(nicsWithLogicalNetwork));
Line 941: 
Line 942:             }
Line 943: 
Line 944:             private void addNullNetwork(List<Network> networks) {
Done - removed and changed the rest of the code to use the null
Line 945:                 if (!isNullNetworkSupported(cluster)) {
Line 946:                     return;
Line 947:                 }
Line 948:                 networks.add(new NullNetwork());


Line 948:                 networks.add(new NullNetwork());
Line 949: 
Line 950:             }
Line 951: 
Line 952:             private boolean isNullNetworkSupported(VDSGroup cluster) {
same
Line 953:                 if (cluster == null || 
cluster.getcompatibility_version() == null) {
Line 954:                     return false;
Line 955:                 }
Line 956: 


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmNetworkCreatingManager.java
Line 21:     public VmNetworkCreatingManager(PostNetworkCreatedCallback 
callback) {
Line 22:         this.callback = callback;
Line 23:     }
Line 24: 
Line 25:     public void createOrUpdateNetworks(final Guid vmId,
Done
Line 26:             final List<NicWithLogicalNetworks> 
nicsWithLogicalNetworks) {
Line 27:         AsyncQuery getVmNicsQuery = new AsyncQuery();
Line 28:         getVmNicsQuery.asyncCallback = new INewAsyncCallback() {
Line 29:             @Override


Line 88: 
Line 89:     public void updateNetworks(final Guid vmId, final 
List<NicWithLogicalNetworks> nicsWithLogicalNetworks) {
Line 90:         ArrayList<VdcActionParametersBase> parameters = 
createAddVmInterfaceParams(vmId, nicsWithLogicalNetworks);
Line 91: 
Line 92:         if (parameters.size() == 0) {
Done
Line 93:             callback.networkCreated();
Line 94:             return;
Line 95:         }
Line 96: 


Line 138: 
Line 139:     public static interface PostNetworkCreatedCallback {
Line 140:         void networkCreated();
Line 141: 
Line 142:         void queryFailed(Object target);
Done
Line 143:     }


-- 
To view, visit http://gerrit.ovirt.org/15102
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8a5b488236ba5a08fc9050ed13a6ed49802769da
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Tomas Jelinek <tjeli...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to