abdullah alamoudi has posted comments on this change.

Change subject: [NO ISSUE][OTH] Enable adding request channel close listener
......................................................................


Patch Set 6:

(8 comments)

https://asterix-gerrit.ics.uci.edu/#/c/1972/5/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/api/IChannelCloseHandler.java
File 
hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/api/IChannelCloseHandler.java:

PS5, Line 26: 
> close -vs- closed; let's pick one
Done


PS5, Line 29: 
> closed -vs- close- let's pick one
Done


PS5, Line 36: 
> task*
Done


PS5, Line 38: 
> rename this to be less generic e.g. handleClosed, to enable classes to impl
Done


https://asterix-gerrit.ics.uci.edu/#/c/1972/5/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/api/IServlet.java
File 
hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/api/IServlet.java:

PS5, Line 50: 
> should we add HttpServer as a method parameter, and return server.getCHanne
Done


https://asterix-gerrit.ics.uci.edu/#/c/1972/5/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServer.java
File 
hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServer.java:

PS5, Line 382:     public ThreadPoolExecutor getExec
> this was to enable extensions to choose a thread pool based on the request,
Done


https://asterix-gerrit.ics.uci.edu/#/c/1972/5/hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServerHandler.java
File 
hyracks-fullstack/hyracks/hyracks-http/src/main/java/org/apache/hyracks/http/server/HttpServerHandler.java:

PS5, Line 105:  ctx
> check*
Done


PS5, Line 103:            final IChannelClosedHandler closeHandler = 
servlet.getChannelCloseHandler(server);
             :             if (closeHandler != null) {
             :                 ctx.channel().closeFuture().addListener(future 
-> closeHandler.channelClosed(server, servlet, task));
             :             }
             :         } catch (RejectedExecutionException e) { // NOSONAR
             :             L
> i think this gets cleaner if the servlet default impl delegates to server
Done


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1972
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I42f1857c0158af6f447282cab8fbd600767b08d5
Gerrit-PatchSet: 6
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: abdullah alamoudi <bamou...@gmail.com>
Gerrit-Reviewer: Anon. E. Moose #1000171
Gerrit-Reviewer: Dmitry Lychagin <dmitry.lycha...@couchbase.com>
Gerrit-Reviewer: Jenkins <jenk...@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Michael Blow <mb...@apache.org>
Gerrit-Reviewer: Murtadha Hubail <mhub...@apache.org>
Gerrit-Reviewer: Till Westmann <ti...@apache.org>
Gerrit-Reviewer: abdullah alamoudi <bamou...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to