[
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)