You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/08/05 13:44:25 UTC
[GitHub] [apisix] dabue opened a new pull request #2000: [change] enabled websocket in route
dabue opened a new pull request #2000:
URL: https://github.com/apache/apisix/pull/2000
change: enabled websocket in route
resolve #1791
### Pre-submission checklist:
*enable websocket
*update the test cases
*update the user's guide
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2000: [change] enabled websocket in route
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2000:
URL: https://github.com/apache/apisix/pull/2000#discussion_r467534447
##########
File path: apisix/schema_def.lua
##########
@@ -465,6 +465,10 @@ _M.service = {
name = {type = "string", maxLength = 50},
desc = {type = "string", maxLength = 256},
script = {type = "string", minLength = 10, maxLength = 102400},
+ enable_websocket = {
Review comment:
we need to update the doc about `service`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on pull request #2000: [change] enabled websocket in route
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #2000:
URL: https://github.com/apache/apisix/pull/2000#issuecomment-671008207
why not supports enable websocket in both route and service?
YuanSheng Wang <no...@github.com> 于 2020年8月7日周五 下午9:55写道：
> Is this a non-backward compatible PR?
>
> yes
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/apisix/pull/2000#issuecomment-670528690>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AGJZBK6HRVUKDOATWDYGL33R7QBTPANCNFSM4PVP55IQ>
> .
>
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2000: [change] enabled websocket in route
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2000:
URL: https://github.com/apache/apisix/pull/2000#discussion_r469373048
##########
File path: apisix/init.lua
##########
@@ -442,15 +442,12 @@ function _M.http_access_phase()
return_direct, route)
end
end
-
- if route.value.upstream and route.value.upstream.enable_websocket then
- enable_websocket = true
- end
end
if enable_websocket then
api_ctx.var.upstream_upgrade = api_ctx.var.http_upgrade
api_ctx.var.upstream_connection = api_ctx.var.http_connection
+ core.log.info("enabled websocket for route: ", route.value.id)
Review comment:
why we need this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2000: [change] enabled websocket in route
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2000:
URL: https://github.com/apache/apisix/pull/2000#issuecomment-669231264
@dabue I think we need some test case about the `enable_websocket` was used in `service` object.
For this changing:
https://github.com/apache/apisix/pull/2000/files#diff-d982d52466e7c93c7b604358339b2a29R378-R380
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] dabue commented on a change in pull request #2000: [change] enabled websocket in route
Posted by GitBox <gi...@apache.org>.
dabue commented on a change in pull request #2000:
URL: https://github.com/apache/apisix/pull/2000#discussion_r469651786
##########
File path: apisix/schema_def.lua
##########
@@ -465,6 +465,10 @@ _M.service = {
name = {type = "string", maxLength = 50},
desc = {type = "string", maxLength = 256},
script = {type = "string", minLength = 10, maxLength = 102400},
+ enable_websocket = {
Review comment:
I have updated the admin-api.md, please check it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on pull request #2000: [change] enabled websocket in route
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #2000:
URL: https://github.com/apache/apisix/pull/2000#issuecomment-672426812
@dabue ping
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2000: [change] enabled websocket in route
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2000:
URL: https://github.com/apache/apisix/pull/2000#issuecomment-672957477
> why not supports enable websocket in both route and service?
That is good if we want to enable `enable_websocket` in the `service` level.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] dabue commented on pull request #2000: [change] enabled websocket in route
Posted by GitBox <gi...@apache.org>.
dabue commented on pull request #2000:
URL: https://github.com/apache/apisix/pull/2000#issuecomment-673189763
> @dabue ping
This function has been supported in route and service. I‘ll update the docs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2000: [change] enabled websocket in route
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2000:
URL: https://github.com/apache/apisix/pull/2000#issuecomment-684525692
@dabue pls rebase your branch
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2000: [change] enabled websocket in route
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2000:
URL: https://github.com/apache/apisix/pull/2000#discussion_r469373353
##########
File path: apisix/init.lua
##########
@@ -442,15 +442,12 @@ function _M.http_access_phase()
return_direct, route)
end
end
-
- if route.value.upstream and route.value.upstream.enable_websocket then
- enable_websocket = true
- end
end
if enable_websocket then
api_ctx.var.upstream_upgrade = api_ctx.var.http_upgrade
api_ctx.var.upstream_connection = api_ctx.var.http_connection
+ core.log.info("enabled websocket for route: ", route.value.id)
Review comment:
wow, I got it, for testing. ignore what I said.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] dabue commented on pull request #2000: [change] enabled websocket in route
Posted by GitBox <gi...@apache.org>.
dabue commented on pull request #2000:
URL: https://github.com/apache/apisix/pull/2000#issuecomment-670995246
ok, I'll update it.
88786912@qq.com
From: YuanSheng Wang
Date: 2020-08-05 22:37
To: apache/apisix
CC: dabue; Mention
Subject: Re: [apache/apisix] [change] enabled websocket in route (#2000)
@dabue I think we need some test case about the enable_websocket was used in service object.
For this changing:
https://github.com/apache/apisix/pull/2000/files#diff-d982d52466e7c93c7b604358339b2a29R378-R380
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] dabue commented on pull request #2000: [change] enabled websocket in route
Posted by GitBox <gi...@apache.org>.
dabue commented on pull request #2000:
URL: https://github.com/apache/apisix/pull/2000#issuecomment-670996685
I have updated it , please help to check it, thx.
88786912@qq.com
From: dabue
Date: 2020-08-09 10:03
To: apache/apisix; apache/apisix
CC: mention
Subject: Re: Re: [apache/apisix] [change] enabled websocket in route (#2000)
ok, I'll update it.
88786912@qq.com
From: YuanSheng Wang
Date: 2020-08-05 22:37
To: apache/apisix
CC: dabue; Mention
Subject: Re: [apache/apisix] [change] enabled websocket in route (#2000)
@dabue I think we need some test case about the enable_websocket was used in service object.
For this changing:
https://github.com/apache/apisix/pull/2000/files#diff-d982d52466e7c93c7b604358339b2a29R378-R380
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2000: [change] enabled websocket in route
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2000:
URL: https://github.com/apache/apisix/pull/2000#issuecomment-670528690
> Is this a non-backward compatible PR?
yes
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org