Copilot commented on code in PR #13884:
URL: https://github.com/apache/cloudstack/pull/13884#discussion_r4324660413
##########
api/src/main/java/com/cloud/storage/VolumeApiService.java:
##########
@@ -61,6 +61,18 @@ public interface VolumeApiService {
true,
ConfigKey.Scope.Zone);
+ ConfigKey<Integer> CopyVolumeWait = new ConfigKey<>("Storage",
Integer.class, "copy.volume.wait", "10800",
+ "In second, timeout for copy volume command", true);
+
+ ConfigKey<Integer> CreateVolumeFromSnapshotWait = new
ConfigKey<>("Storage", Integer.class, "create.volume.from.snapshot.wait",
"10800",
+ "In second, timeout for creating volume from snapshot", true);
Review Comment:
The description text should use the plural form for units. Consider changing
'In second' to 'In seconds' for clarity.
##########
engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/ObjectStoreDaoImpl.java:
##########
Review Comment:
This introduces yet another local `ConfigKey` definition for
`detail.batch.query.size` (similar local definitions also exist in other DAO
classes). Duplicating the same key in multiple places increases the risk of
metadata drift (category/description/scope/defaults diverging). A more
maintainable approach is to define this key once in a shared, dependency-safe
module (e.g., engine/components-api or engine/api) and reference it everywhere.
##########
engine/components-api/src/main/java/com/cloud/network/NetworkStateListener.java:
##########
Review Comment:
The description for `publish.resource.state.events` is inaccurate: it refers
to 'alert events', but this listener publishes *resource state* events. Please
update the `ConfigKey` description to match what is actually being published.
--
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]
