dlmarion commented on code in PR #53:
URL: 
https://github.com/apache/accumulo-classloaders/pull/53#discussion_r2722527694


##########
modules/local-caching-classloader/src/main/java/org/apache/accumulo/classloader/lcc/definition/ContextDefinition.java:
##########
@@ -51,14 +54,55 @@
 @AutoService(KeywordExecutable.class)
 public class ContextDefinition implements KeywordExecutable {
 
-  static class Opts extends Help {
-    @Parameter(names = {"-i", "--interval"}, required = true,
-        description = "monitor interval (in seconds)", arity = 1, order = 1)
-    int monitorInterval;
+  static class Opts {
+    @Parameter(names = {"-i", "--interval"}, required = false,
+        description = "monitor interval (in seconds, default: 300)", arity = 
1, order = 1)
+    int monitorInterval = 300; // 5 minutes

Review Comment:
   I don't think we should specify a default here. I'd rather fail if it's not 
supplied. That makes the user think about how often they are hitting the 
context definition location and what kind of load they are putting on that 
resource given the number of processes that will be hitting it.



-- 
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]

Reply via email to