Michael Blow has posted comments on this change. Change subject: Overhaul of Hyracks configuration management. ......................................................................
Patch Set 4: (7 comments) https://asterix-gerrit.ics.uci.edu/#/c/336/4/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/IPropertyInterpreter.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/IPropertyInterpreter.java: Line 21: import org.apache.asterix.common.configuration.Property; Unused import https://asterix-gerrit.ics.uci.edu/#/c/336/4/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/PropertyInterpreters.java File asterixdb/asterix-common/src/main/java/org/apache/asterix/common/config/PropertyInterpreters.java: Line 23: import org.apache.asterix.common.configuration.Property; Unsed import https://asterix-gerrit.ics.uci.edu/#/c/336/4/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/application/IApplicationConfig.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/application/IApplicationConfig.java: Line 26: int getInt(String section, String key); Do we have a convention for parameter names? I think camel-case is pretty prevalent. https://asterix-gerrit.ics.uci.edu/#/c/336/4/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/ClusterControllerService.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/ClusterControllerService.java: Line 283: // QQQ reasonable? Is IPv6 not supported by Hyracks? Perhaps this should be InetAddress.getLoopbackAddress().toString() to future-proof? https://asterix-gerrit.ics.uci.edu/#/c/336/4/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/TriggerNCWork.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/TriggerNCWork.java: Line 57: oos.writeUTF("hyncmagic"); Constant? https://asterix-gerrit.ics.uci.edu/#/c/336/4/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/NCDriver.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-nc/src/main/java/org/apache/hyracks/control/nc/NCDriver.java: Line 38: System.err.println(e.getMessage()); In general, Throwable.getMessage() does not yield useful information. Best to use toString() as normally the message (if present) only makes sense in context of the exception class name. Understood you didn't introduce this :-). https://asterix-gerrit.ics.uci.edu/#/c/336/4/hyracks-fullstack/hyracks/hyracks-control/hyracks-nc-service/src/main/java/org/apache/hyracks/control/nc/service/NCService.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-nc-service/src/main/java/org/apache/hyracks/control/nc/service/NCService.java: Line 93: // QQQ inheriting probably isn't right What will you do with the output? Is there any use of stdin by the NC? -- To view, visit https://asterix-gerrit.ics.uci.edu/336 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie3027c8c839f25ea858790bd3340187f4b11f212 Gerrit-PatchSet: 4 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Chris Hillery <c...@lambda.nu> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Blow <michael.b...@couchbase.com> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-HasComments: Yes
