GabrielBrascher commented on a change in pull request #4190:
URL: https://github.com/apache/cloudstack/pull/4190#discussion_r485981641
##########
File path:
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java
##########
@@ -2475,6 +2475,21 @@ public Network doInTransaction(final TransactionStatus
status) {
return network;
}
+ /**
+ * Encodes VLAN/VXLAN ID into a Broadcast URI according to the isolation
method from the Physical Network.
+ * @return Broadcast URI, e.g. 'vlan://vlan_ID' or 'vxlan://vlxan_ID'
+ */
+ protected URI encodeVlanIdIntoBroadcastUri(String vlanId, PhysicalNetwork
pNtwk) {
+ if(StringUtils.isNotBlank(pNtwk.getIsolationMethods().get(0))) {
Review comment:
@rhtyd thanks for the review :-)
Answering your question: I did not add null verification on the respective
parameter due to the fact that both methods that call this one already validate
the private network
1. the physical network `pNtwk` is validated at
[NetworkOrchestrator.java#L2196](https://github.com/apache/cloudstack/blob/2c3a41c7a63dfd1dd271578eacec26442ee039f9/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java#L2196);
2. the other method that calls `encodeVlanIdIntoBroadcastUri` also validates
the private network at this line
[NetworkOrchestrator.java#L2397](https://github.com/apache/cloudstack/blob/2c3a41c7a63dfd1dd271578eacec26442ee039f9/engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java#L2397).
However, considering that this method could be reused, I have no problem in
adding null validations.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
