[ 
https://issues.apache.org/jira/browse/KNOX-776?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16064272#comment-16064272
 ] 

Sandeep More edited comment on KNOX-776 at 6/27/17 5:02 AM:
------------------------------------------------------------

Hello [~jesus.alv],
First of all apologies for the delay ! and thanks for the patch this would be a 
great improvement to websocket functionality for knox.
Following are some of my minor review comments, some are general cleanup, feel 
free to ignore them if you would like, I thought if you are updating the file 
we might as well fix it :)

* GatewayWebsocketHandler
** Line #60 - unused REGEX_SPLIT_CLUSTER_NAME (not part of the patch but just a 
general cleanup)
** Line 202 - dangling ; (not part of the patch but just a general cleanup)
** Line 209 - angle brackets preferred after if statements.
** What happens if no context is used, i.e. 
{code}
  String[] pathService = path.split(REGEX_SPLIT_SERVICE_PATH); 
{code}
  here you would likely get a NPE at line #210 - a quick solution would be to 
check for null, I think.
  
* WebsocketRewriteTest Class
** You could simply move the test into existing classes such as 
WebsocketMultipleConnectionTest. 
  This would save a lot of time during test runs by preventing starting and 
stopping test gateway and websocket servers.
** Also if possible can you change the test from multiple connection to just 
one connection, this would save some time and cpu
  while building :) 

Thanks a lot for the patch, I will try to test it and get it as quickly as I 
can for 0.13.0


was (Author: smore):
Hello [~jesus.alv],
First of all apologies for the delay ! and thanks for the patch this would be a 
great improvement to websocket functionality for knox.
Following are some of my minor review comments, some are general cleanup, feel 
free to ignore them if you would like, I thought if you are updating the file 
we might as well fix it :)

* GatewayWebsocketHandler
** Line #60 - unused REGEX_SPLIT_CLUSTER_NAME (not part of the patch but just a 
general cleanup)
** Line 202 - dangling ; (not part of the patch but just a general cleanup)
** Line 209 - angle brackets preferred after if statements.
** What happens if no context is used, i.e. 
{code}
  String[] pathService = path.split(REGEX_SPLIT_SERVICE_PATH); 
{code}
  here you would likely get a NPE at line #210 - a quick solution would be to 
check for null, I think.
  
* WebsocketRewriteTest Class
** You could simply move the test into existing classes such as 
WebsocketMultipleConnectionTest. 
  This would save a lot of time during test runs by preventing starting and 
stoping test gateway and websocket servers.
** Also if possible can you change the test from multiple connection to just 
one connection, this would save some time and cpu
  while building :) 

Thanks a lot for the patch, I will try to test it and get it as quickly as I 
can for 0.13.0

> Rewrite rule handling for Websockets
> ------------------------------------
>
>                 Key: KNOX-776
>                 URL: https://issues.apache.org/jira/browse/KNOX-776
>             Project: Apache Knox
>          Issue Type: Bug
>          Components: Server
>            Reporter: Sandeep More
>            Assignee: Jesus Alvarez
>             Fix For: 0.13.0
>
>         Attachments: KNOX-776.patch
>
>
> Currently we simply proxy websocket payload we should support some form of 
> rewrite rule handling.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to