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/09 06:07:01 UTC

[GitHub] [apisix] membphis opened a new pull request #2029: bugfix: only set one response header when enabled `enable_debug=true`

membphis opened a new pull request #2029:
URL: https://github.com/apache/apisix/pull/2029


       and `global rules`.
   
   ### What this PR does / why we need it:
   <!--- Why is this change required? What problem does it solve? -->
   <!--- If it fixes an open issue, please link to the issue here. -->
   
   fix #1667
   
   ### Pre-submission checklist:
   
   * [ ] Did you explain what problem does this PR solve? Or what new features have been added?
   * [ ] Have you added corresponding test cases?
   * [ ] Have you modified the corresponding document?
   * [ ] Is this PR backward compatible?
   


----------------------------------------------------------------
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] spacewander commented on a change in pull request #2029: bugfix: only set one response header when enabled `enable_debug=true`

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2029:
URL: https://github.com/apache/apisix/pull/2029#discussion_r500709172



##########
File path: apisix/plugin.lua
##########
@@ -228,13 +228,34 @@ function _M.api_routes()
 end
 
 
-function _M.filter(user_route, plugins)
+local function set_response_header_by_debug_flag(plugins, dry_run)
+    if dry_run then

Review comment:
       I think we can use `ngx.headers_sent` instead of passing flag manually.




----------------------------------------------------------------
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] spacewander commented on a change in pull request #2029: bugfix: only set one response header when enabled `enable_debug=true`

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2029:
URL: https://github.com/apache/apisix/pull/2029#discussion_r500713424



##########
File path: apisix/plugin.lua
##########
@@ -228,13 +228,34 @@ function _M.api_routes()
 end
 
 
-function _M.filter(user_route, plugins)
+local function set_response_header_by_debug_flag(plugins, dry_run)
+    if dry_run then

Review comment:
       And to avoid the conflict between global rule / route rule, it would be better to use `add_header`.
   Maybe we still need to pass the `global` flag to indicate if the current filtering rule is global or not?




----------------------------------------------------------------
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 closed pull request #2029: bugfix: only set one response header when enabled `enable_debug=true`

Posted by GitBox <gi...@apache.org>.
membphis closed pull request #2029:
URL: https://github.com/apache/apisix/pull/2029


   


----------------------------------------------------------------
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 #2029: bugfix: only set one response header when enabled `enable_debug=true`

Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2029:
URL: https://github.com/apache/apisix/pull/2029#discussion_r540632723



##########
File path: apisix/plugin.lua
##########
@@ -228,13 +228,34 @@ function _M.api_routes()
 end
 
 
-function _M.filter(user_route, plugins)
+local function set_response_header_by_debug_flag(plugins, dry_run)
+    if dry_run then

Review comment:
       > I think we can use `ngx.headers_sent` instead of passing flag manually.
   
   yes, this is a much better way. I gonna close this PR.




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