Martin Mucha has posted comments on this change.

Change subject: core,restapi: allow indetifying network by its name when adding 
network attachment allow indetifying network by its name.
......................................................................


Patch Set 16:

(3 comments)

https://gerrit.ovirt.org/#/c/40359/16/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/NetworkIdNetworkNameCompleter.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/NetworkIdNetworkNameCompleter.java:

Line 36:         Guid networkId = networkAttachment.getNetworkId();
Line 37:         String networkName = networkAttachment.getNetworkName();
Line 38: 
Line 39:         if (networkId == null && networkName == null) {
Line 40:             throw new IllegalArgumentException("network name or 
network id must be set.");
> Noticed it while reviewing another patch- you should have candoaction for t
completion skipped.Done.
Line 41:         }
Line 42: 
Line 43:         if (networkName != null) {
Line 44:             Network network = getNetworkByName(networkName, 
clusterNetworks);


Line 46:                 if (networkId == null) {
Line 47:                     networkAttachment.setNetworkId(network.getId());
Line 48:                 } else {
Line 49:                     if (!network.getId().equals(networkId)) {
Line 50:                         throw new IllegalArgumentException("Both nicId 
and nicName was specified, "
> Noticed it while reviewing another patch- you should have candoaction for t
Done
Line 51:                                 + "but they does not denote same 
nic.");
Line 52:                     }
Line 53:                 }
Line 54:             }


Line 46:                 if (networkId == null) {
Line 47:                     networkAttachment.setNetworkId(network.getId());
Line 48:                 } else {
Line 49:                     if (!network.getId().equals(networkId)) {
Line 50:                         throw new IllegalArgumentException("Both nicId 
and nicName was specified, "
> I suggest here just ignoring it, and in the validator make sure the name an
Done
Line 51:                                 + "but they does not denote same 
nic.");
Line 52:                     }
Line 53:                 }
Line 54:             }


-- 
To view, visit https://gerrit.ovirt.org/40359
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2b692915593a86e9ea4b8faa8056cbcafefe328e
Gerrit-PatchSet: 16
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Martin Mucha <mmu...@redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Mucha <mmu...@redhat.com>
Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@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