Alona Kaplan has posted comments on this change.

Change subject: webadmin: add profiles instead of networks to vnic and vm 
dialogs
......................................................................


Patch Set 19:

(13 comments)

....................................................
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/CommonApplicationMessages.java
Line 104: 
Line 105:     @DefaultMessage("default: {0}")
Line 106:     String defaultTimeZoneCaption(String currentDefault);
Line 107: 
Line 108:     @DefaultMessage("VM has {0} Nics. Assign them to the Profiles.")
Done
Line 109:     String assignNicsToProfilesPlural(int numOfNics);
Line 110: 
Line 111:     @DefaultMessage("VM has 1 Nic. Assign it to a Profile.")
Line 112:     String assignNicsToProfilesSingular();


....................................................
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/network/LogicalNetworkEditor.java
Line 18: import com.google.gwt.user.client.TakesValue;
Line 19: import com.google.gwt.user.client.ui.Composite;
Line 20: import com.google.gwt.user.client.ui.Widget;
Line 21: 
Line 22: public class LogicalNetworkEditor extends Composite implements 
IsEditor<TakesValueEditor<VnicInstanceType>>, TakesValue<VnicInstanceType>, 
HasElementId {
The items are not going to be changed after initializing the editor. So in this 
phase it would be an overhead to implement TakesConstrainedValue.

And also, this is not in the scope of this patch:)
Line 23: 
Line 24:     interface WidgetUiBinder extends UiBinder<Widget, 
LogicalNetworkEditor> {
Line 25:         WidgetUiBinder uiBinder = GWT.create(WidgetUiBinder.class);
Line 26:     }


....................................................
File 
frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/networkinterface/NetworkInterfacePopupWidget.java
Line 65: 
Line 66:     @UiField(provided = true)
Line 67:     @Path(value = "profile.selectedItem")
Line 68:     @WithElementId("profile")
Line 69:     public ListModelTypeAheadListBoxEditor<Object> profileEditor;
The LogicalNetworkEditor is strongly binded to vnic instance type and can't be 
reused here.
Line 70: 
Line 71:     @UiField(provided = true)
Line 72:     @Path("nicType.selectedItem")
Line 73:     @WithElementId("nicType")


Line 168:     }
Line 169: 
Line 170:     @SuppressWarnings({ "rawtypes", "unchecked" })
Line 171:     private void initManualWidgets() {
Line 172:         profileEditor = new ListModelTypeAheadListBoxEditor<Object>(
Done
Line 173:                 new 
ListModelTypeAheadListBoxEditor.NullSafeSuggestBoxRenderer<Object>() {
Line 174: 
Line 175:                     @Override
Line 176:                     public String getReplacementStringNullSafe(Object 
data) {


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/EditProfileBehavior.java
Line 10: 
Line 11: public class EditProfileBehavior extends ProfileBehavior {
Line 12: 
Line 13:     @Override
Line 14:     public void initSelectedProfile(ListModel profileList, 
VmNetworkInterface networkInterface) {
This method is called only once when opening the edit dialog.
I don't think it worth to maintain a map for it.
Line 15:         List<VnicProfileView> profiles = (List<VnicProfileView>) 
profileList.getItems();
Line 16:         profiles = profiles == null ? new ArrayList<VnicProfileView>() 
: profiles;
Line 17: 
Line 18:         if (networkInterface.getVnicProfileId() == null) {


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/NewProfileBehavior.java
Line 18:     public void initSelectedProfile(ListModel profileList, 
VmNetworkInterface networkInterface) {
Line 19:         List<VnicProfileView> profiles = (List<VnicProfileView>) 
profileList.getItems();
Line 20:         profiles = profiles == null ? new ArrayList<VnicProfileView>() 
: profiles;
Line 21:         for (VnicProfileView profile : profiles) {
Line 22:             if (ENGINE_NETWORK_NAME != null && profile != null && 
profile.getNetworkName() != null
1. The key of the map you"ve suggested is the profile name. Here I need to find 
the profile by the network name. So the map wouldn't help me here.
2. You are right. Removed this check.
Line 23:                     && 
ENGINE_NETWORK_NAME.equals(profile.getNetworkName())) {
Line 24:                 profileList.setSelectedItem(profile);
Line 25:                 return;
Line 26:             }


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/ProfileBehavior.java
Line 28:         profilesQuery.converterCallback = new IAsyncConverter() {
Line 29: 
Line 30:             @Override
Line 31:             public Object Convert(Object returnValue, AsyncQuery 
asyncQuery) {
Line 32:                 ArrayList<Network> clusterNetworks = 
(ArrayList<Network>) asyncQuery.getModel();
Done
Line 33: 
Line 34:                 ProfileBehavior.this.clusterNetworks = clusterNetworks;
Line 35: 
Line 36:                 ArrayList<VnicProfileView> vnicProfiles = new 
ArrayList<VnicProfileView>();


Line 38:                 if (returnValue == null) {
Line 39:                     return vnicProfiles;
Line 40:                 }
Line 41: 
Line 42:                 for (VnicProfileView vnicProfile : 
(ArrayList<VnicProfileView>) returnValue) {
Changed the cast to List instead of ArrayList.
I prefer not to use iterator. I think foreach is much more readable.
Line 43:                     Network network = 
findNetworkById(vnicProfile.getNetworkId());
Line 44:                     if (network != null && network.isVmNetwork()) {
Line 45:                         vnicProfiles.add(vnicProfile);
Line 46:                     }


Line 50:                 if (hotUpdateSupported) {
Line 51:                     vnicProfiles.add(null);
Line 52:                 }
Line 53: 
Line 54:                 Collections.sort(vnicProfiles, new 
Comparator<VnicProfileView>() {
You are right:) That's why I did it in patch http://gerrit.ovirt.org/#/c/17574/
Line 55: 
Line 56:                     private LexoNumericComparator lexoNumeric = new 
LexoNumericComparator();
Line 57: 
Line 58:                     @Override


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VmInterfaceCreatingManager.java
Line 44: 
Line 45:     /**
Line 46:      * Used mainly when the VM is created from the template. If the Vm 
has been created but without NICs, new
Line 47:      * ones are created according to the vnicInstanceTypes. If the VM 
is created with nics - e.g. by copying from template update they're profiles
Line 48:      * as edited by the user (again, according to the 
vnicInstanceTypes).
Fixed the typo. It is formatted, the problem is with gerrit's display.
Line 49:      *
Line 50:      * @param vmId The ID of the VM
Line 51:      * @param vnicInstanceTypes list of nics as edited in the window
Line 52:      */


Line 65:                     // there are some vnics created - update according 
to the setup in the window
Line 66:                     ArrayList<VdcActionParametersBase> parameters = 
new ArrayList<VdcActionParametersBase>();
Line 67: 
Line 68:                     for (VmNetworkInterface created : createdNics) {
Line 69:                         for (VnicInstanceType edited : 
vnicInstanceTypes) {
This class was renamed from VmNetworkCreatingManager.java to 
VmInterfaceCreatingManager.java. And was adapted to use profiles.
Most of the code in thus class is not in the scope of this patch.
Line 70:                             // can not use getId() because they have 
different IDs - one is already created, one is not
Line 71:                             // yet
Line 72:                             boolean sameNic = 
edited.getNetworkInterface().getName().equals(created.getName());
Line 73: 


Line 68:                     for (VmNetworkInterface created : createdNics) {
Line 69:                         for (VnicInstanceType edited : 
vnicInstanceTypes) {
Line 70:                             // can not use getId() because they have 
different IDs - one is already created, one is not
Line 71:                             // yet
Line 72:                             boolean sameNic = 
edited.getNetworkInterface().getName().equals(created.getName());
This is not correct. You can create two vnics with the same name and different 
cases.
Line 73: 
Line 74:                             boolean bothProfilesNull = 
created.getVnicProfileId() == null && 
edited.getNetworkInterface().getVnicProfileId() == null;
Line 75: 
Line 76:                             boolean sameProfiles = 
created.getVnicProfileId() != null && 
created.getVnicProfileId().equals(edited.getNetworkInterface().getVnicProfileId());


....................................................
File 
frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/vms/VnicInstanceType.java
Line 2: 
Line 3: import 
org.ovirt.engine.core.common.businessentities.network.VmNetworkInterface;
Line 4: import org.ovirt.engine.ui.uicommonweb.models.ListModel;
Line 5: 
Line 6: public class VnicInstanceType extends ListModel {
It is vnic representation to be used as instance type for the vm.
Line 7: 
Line 8:     private VmNetworkInterface networkInterface;
Line 9: 
Line 10:     public VnicInstanceType(VmNetworkInterface networkInterface) {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I00ea69609acf8a8bfeb0a5357489141f726b6323
Gerrit-PatchSet: 19
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Lior Vernia <lver...@redhat.com>
Gerrit-Reviewer: Livnat Peer <lp...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
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

Reply via email to