Chris Hillery has posted comments on this change.

Change subject: Overhaul of Hyracks configuration management.
......................................................................


Patch Set 4:

(20 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
Done


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
Done


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 14:  */
> This it the wrong license header.
Done


Line 21:  * Created by ceej on 6/26/15.
> Ideally there's no author attribution in ASF source files (yes, we do have 
Done


Line 26:     int getInt(String section, String key);
> This is referencing default_value, which should be defaultValue.
Done


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?
> probably InetAddress.getLoopbackAddress().getHostAddress().
Done here. Also fixed CCConfig and NCConfig. FYI there are many other places 
throughout the code which reference 127.0.0.1 explicitly, although I believe 
most are in test cases. I filed 
https://issues.apache.org/jira/browse/ASTERIXDB-1420 to worry about that.


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 14:  */
> Wrong license
Done


Line 29:  * Created by ceej on 6/18/15.
> Please no author attribution.
Done


Line 57:                         oos.writeUTF("hyncmagic");
> Move this to a constant field in this file, and include comment that it mus
Done


https://asterix-gerrit.ics.uci.edu/#/c/336/4/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/application/IniApplicationConfig.java
File 
hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/application/IniApplicationConfig.java:

Line 14:  */
> Wrong license
Done


Line 23:  * Created by ceej on 6/26/15.
> Please no author attribution.
Done


https://asterix-gerrit.ics.uci.edu/#/c/336/4/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/controllers/IniUtils.java
File 
hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/controllers/IniUtils.java:

Line 14:  */
> Wrong license.
Done


Line 24:  * Created by ceej on 7/23/15.
> Please no author attribution.
Done


https://asterix-gerrit.ics.uci.edu/#/c/336/4/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/controllers/NCConfig.java
File 
hyracks-fullstack/hyracks/hyracks-control/hyracks-control-common/src/main/java/org/apache/hyracks/control/common/controllers/NCConfig.java:

Line 154:     
> WS
Done


Line 166:         
> WS
Done


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());
> Probably should just remove this line, since e.printStrackTrace() does all 
Done


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 14:  */
> Wrong license
Done


Line 36:  * Created by ceej on 5/28/15.
> Please no author attribution.
Done


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/NCServiceConfig.java
File 
hyracks-fullstack/hyracks/hyracks-control/hyracks-nc-service/src/main/java/org/apache/hyracks/control/nc/service/NCServiceConfig.java:

Line 14:  */
> Wrong license
Done


Line 20:  * Created by ceej on 5/28/15.
> Please no author attribution.
Done


-- 
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 <[email protected]>
Gerrit-Reviewer: Chris Hillery <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Michael Blow <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-HasComments: Yes

Reply via email to