Copilot commented on code in PR #14055:
URL: https://github.com/apache/cloudstack/pull/14055#discussion_r3931343265
##########
server/src/main/java/com/cloud/api/query/dao/UserVmJoinDaoImpl.java:
##########
@@ -447,6 +448,10 @@ public UserVmResponse newUserVmResponse(ResponseView view,
String objectName, Us
.collect(Collectors.toList());
nicResponse.setExtraDhcpOptions(nicExtraDhcpOptionResponses);
+ NicDetailVO nicNetworkRateDetail =
ApiDBUtils.findNicDetailByName(userVm.getNicId(), ApiConstants.NETWORKRATE);
+ if (nicNetworkRateDetail != null) {
+
nicResponse.setNetworkRate(nicNetworkRateDetail.getValue());
+ }
Review Comment:
This adds a DAO lookup per NIC while building VM responses
(`findNicDetailByName(...)` inside the NIC loop). For VMs with multiple NICs
(and especially listVms across many VMs), this can introduce an N+1 query
pattern and increase DB load.
##########
api/src/main/java/org/apache/cloudstack/api/response/NetworkResponse.java:
##########
@@ -311,6 +311,10 @@ public class NetworkResponse extends
BaseResponseWithAssociatedNetwork implement
@Param(description = "MTU configured on the network VR's private
interfaces")
private Integer privateMtu;
+ @SerializedName(ApiConstants.NETWORKRATE)
+ @Param(description = "Network rate (in Mb/s) configured for the Guest
interface of this network", since = "4.24.0")
+ private String networkRate;
Review Comment:
PR description says `NetworkResponse.networkrate` is `since = "4.21.0"`, but
the annotation here declares `since = "4.24.0"`. Please align the `since`
version with the intended release/API contract (either update this annotation
or correct the PR description).
##########
server/src/main/java/com/cloud/vm/UserVmManagerImpl.java:
##########
@@ -524,6 +525,8 @@ public class UserVmManagerImpl extends ManagerBase
implements UserVmManager, Vir
@Inject
private NicDao _nicDao;
@Inject
+ private NicDetailsDao nicDetailsDao;
+ @Inject
Review Comment:
Injected DAO field name `nicDetailsDao` doesn’t follow the surrounding
`_camelCase` convention used for injected dependencies in this class (e.g.
`_nicDao`, `_rulesMgr`). This inconsistency makes the class harder to scan and
is easy to avoid by renaming to match the established pattern.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
