Michael Blow has posted comments on this change. Change subject: Replace Servlets with Netty Based HTTP Servers ......................................................................
Patch Set 3: (7 comments) https://asterix-gerrit.ics.uci.edu/#/c/1429/3/asterixdb/asterix-app/pom.xml File asterixdb/asterix-app/pom.xml: Line 347: <version>2.2.2</version> inherit (remove) version https://asterix-gerrit.ics.uci.edu/#/c/1429/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ApiLet.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ApiLet.java: Line 116: IHyracksDataset hds; Consider combining declaration & assignment for these two? (hcc/hds) https://asterix-gerrit.ics.uci.edu/#/c/1429/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ClusterApiLet.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/ClusterApiLet.java: Line 134: requestURL.append(request.getHeader(HttpHeaderNames.HOST.toString())); superflous toString() https://asterix-gerrit.ics.uci.edu/#/c/1429/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceLet.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryServiceLet.java: Line 512: hcc = (IHyracksClientConnection) ctx.get(HYRACKS_CONNECTION_ATTR); Who sets HYRACKS_CONNECTION_ATTR if null above? https://asterix-gerrit.ics.uci.edu/#/c/1429/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryStatusApiLet.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/QueryStatusApiLet.java: Line 81: hds = new HyracksDataset(hcc, ResultReader.FRAME_SIZE, ResultReader.NUM_READERS); This is only use of hcc, consider inline or move var decl & set into this block? https://asterix-gerrit.ics.uci.edu/#/c/1429/3/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/RestApiLet.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/http/server/RestApiLet.java: Line 176: IHyracksDataset hds; Consider combining declaration & assignment for these two? (hcc/hds) https://asterix-gerrit.ics.uci.edu/#/c/1429/3/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/AbstractLet.java File hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/AbstractLet.java: Line 23: public abstract class AbstractLet implements IServLet { ServLet/Let consistent naming? (applies to all in change) -- To view, visit https://asterix-gerrit.ics.uci.edu/1429 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3d552d4eb8c868535ca4c41cbcf7e352217b18ae Gerrit-PatchSet: 3 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com> Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu> Gerrit-Reviewer: Michael Blow <mb...@apache.org> Gerrit-Reviewer: Till Westmann <ti...@apache.org> Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com> Gerrit-HasComments: Yes
