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 2022/04/27 07:47:38 UTC
[GitHub] [apisix] qihaiyan opened a new pull request, #6949: api-breaker plugin returns default response body
qihaiyan opened a new pull request, #6949:
URL: https://github.com/apache/apisix/pull/6949
### Description
<!-- Please include a summary of the change and which issue is fixed. -->
<!-- Please also include relevant motivation and context. -->
When upstream is in the unhealthy state, the api-breaker returns unhealthy.http_statuses only, without a response body.
In order to be more compatible to the client, return a default response body is useful.
Fixes #6923
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [x] I have added tests corresponding to this change
- [x] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
<!--
Note
1. Mark the PR as draft until it's ready to be reviewed.
2. Always add/update tests for any changes unless you have a good reason.
3. Always update the documentation to reflect the changes made in the PR.
4. Make a new commit to resolve conversations instead of `push -f`.
5. To resolve merge conflicts, merge master instead of rebasing.
6. Use "request review" to notify the reviewer after making changes.
7. Only a reviewer can mark a conversation as resolved.
-->
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r860451191
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
> This problem cannot be solved even if we provide this field. People have to know how to configure it.
>
> For these kinds of returning body, I think we may need a unified way to handle the `Content-Type`.
A unified way is good, but it's not that easy. Modern API returns application/json, but some legacy API returns application/soap+xml like SOAP.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r862463028
##########
apisix/plugins/api-breaker.lua:
##########
@@ -158,6 +165,13 @@ function _M.access(conf, ctx)
-- breaker
if lasttime + breaker_time >= ngx.time() then
+ if conf.break_response_body then
+ core.response.clear_header_as_body_modified()
Review Comment:
We don't need this if the body is not from the upstream
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r867478450
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
Yes. And we should use an array so multiple response headers with the same name can be used.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r860902040
##########
apisix/plugins/api-breaker.lua:
##########
@@ -158,6 +165,11 @@ function _M.access(conf, ctx)
-- breaker
if lasttime + breaker_time >= ngx.time() then
+ if conf.break_response_body then
+ core.response.clear_header_as_body_modified()
+ ngx.header["Content-Type"] = conf.break_response_content_type
Review Comment:
```suggestion
if conf.break_response_content_type then
core.response.set_header("Content-Type", conf.break_response_content_type)
end
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r861649438
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
> Fair enough. But would you like to submit an issue to track the content-type field for other plugins?
Ok, i'll browse all the plugins which should be with a content-type field and submit an issue with a checklist.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
tokers commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r861603580
##########
docs/en/latest/plugins/api-breaker.md:
##########
@@ -44,6 +44,8 @@ In an unhealthy state, when a request is forwarded to an upstream service and th
| Name | Type | Requirement | Default | Valid | Description |
| ----------------------- | ------------- | ----------- | -------- | --------------- | --------------------------------------------------------------------------- |
| break_response_code | integer | required | | [200, ..., 599] | Return error code when unhealthy |
+| break_response_body | string | optional | | | Return response body when unhealthy |
+| break_response_content_type | string | optional | application/json | | Return body's content type when unhealthy |
Review Comment:
```suggestion
| break_response_content_type | string | optional | application/json | | Return body's content type when unhealthy. This field is in effective only if `break_response_body` is configured. |
```
##########
docs/zh/latest/plugins/api-breaker.md:
##########
@@ -44,6 +44,8 @@ title: api-breaker
| 名称 | 类型 | 必选项 | 默认值 | 有效值 | 描述 |
| ----------------------- | -------------- | ------ | ---------- | --------------- | -------------------------------- |
| break_response_code | integer | 必须 | 无 | [200, ..., 599] | 不健康返回错误码 |
+| break_response_body | string | 可选 | 无 | | 不健康返回报文 |
+| break_response_content_type | string | 可选 | application/json | | 不健康返回 Content-Type |
Review Comment:
```suggestion
| break_response_content_type | string | 可选 | application/json | | 不健康返回报文的内容类型，该字段仅在 `break_response_body` 被配置时生效 |
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r859599269
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
If the body is json, and the content-type is text/html, some APP clients may report error, and the terminal user would be affected.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r867427163
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
> As for this plugin, I would suggest adding a field to add any response headers instead of just content-type.
Can we use the ```headers``` field to represent any response headers? I found the ```echo``` plugin and also the ```response-rewrite``` plugin using this attribute. @spacewander
Name | Type | Requirement | Default | Valid | Description
-- | -- | -- | -- | -- | --
headers | array | optional | | | New headers for response. Using an array so multiple response headers with the same name can be used
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r860950802
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
I think the API owner should be responsible to configure route on the apigageway correctly , it's part of the developing work, and only the API owner knows what the correct Content-Type should be. If there's no way to config the Content-Type correctly, when break state is open, the client APP used this API may crash because of the unexpected Content-Type.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on PR #6949:
URL: https://github.com/apache/apisix/pull/6949#issuecomment-1112888252
Can you help to check why the chaos-test failed? @tzssangglass
https://github.com/apache/apisix/runs/6222619168?check_suite_focus=true
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r868073063
##########
apisix/plugins/api-breaker.lua:
##########
@@ -120,7 +132,12 @@ local _M = {
function _M.check_schema(conf)
- return core.schema.check(schema, conf)
+ local ok, err = core.schema.check(schema, conf)
Review Comment:
why do we change this? I can't get your point.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r862465125
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
As for this plugin, I would suggest adding a field to add any response headers instead of just content-type.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r862619029
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
> As for this plugin, I would suggest adding a field to add any response headers instead of just content-type.
Is there any other plugin with such a field? I'm not sure of which field is suitable for representing any response headers.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
spacewander merged PR #6949:
URL: https://github.com/apache/apisix/pull/6949
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r862619029
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
> As for this plugin, I would suggest adding a field to add any response headers instead of just content-type.
Is there any other plugin with such a field? I'm not sure of which field is suitable for representing any response headers. @spacewander
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on PR #6949:
URL: https://github.com/apache/apisix/pull/6949#issuecomment-1112219016
Hi @qihaiyan , can we add a test cases to cover `break_response_content_type` ?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
tokers commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r860358578
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
This problem cannot be solved even if we provide this field. People have to know how to configure it.
For these kinds of returning body, I think we may need a unified way to handle the `Content-Type`.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
tokers commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r859563078
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
Apache APISIX has a few plugins that define the response body but without setting the content type.
I'm thinking about that, if we need to keep the behaviors consistence?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on PR #6949:
URL: https://github.com/apache/apisix/pull/6949#issuecomment-1112235185
> Hi @qihaiyan , can we add a test cases to cover `break_response_content_type` ?
Sure, i'll add the test cases tomorrow.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
spacewander commented on PR #6949:
URL: https://github.com/apache/apisix/pull/6949#issuecomment-1112915069
> Can you help to check why the chaos-test failed? @tzssangglass https://github.com/apache/apisix/runs/6222619168?check_suite_focus=true 
The chaos test is not so stable. I have rerun 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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r862465071
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
> Ok, i'll browse all the plugins which should be with a content-type field and submit an issue with a checklist.
It is not a good idea, as this check won't fix the non-open-source plugins.
One solution is to guess:
1. according to the response body: https://github.com/apache/apisix/blob/cc88a8db0ba64e1cb351e389ca5ec64c516073d6/apisix/control/router.lua#L81 or using https://github.com/spacewander/lua-resty-mime-sniff
2. according to the Accept header or the ext part of the uri
Another solution is to configure it in the route
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on PR #6949:
URL: https://github.com/apache/apisix/pull/6949#issuecomment-1113015346
> The chaos test is not so stable. I have rerun it.
ok, thanks.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
tokers commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r861600745
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
Fair enough. But would you like to submit an issue to track the content-type field for other plugins?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r867427163
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
> As for this plugin, I would suggest adding a field to add any response headers instead of just content-type.
Can we use the ```headers``` field to represent any response headers? I found the ```echo``` plugin and also the ```response-rewrite``` plugin using this attribute. @spacewander
Name | Type | Requirement | Default | Valid | Description
-- | -- | -- | -- | -- | --
headers | object | optional | | | New headers for response
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r868018757
##########
apisix/plugins/api-breaker.lua:
##########
@@ -158,6 +165,13 @@ function _M.access(conf, ctx)
-- breaker
if lasttime + breaker_time >= ngx.time() then
+ if conf.break_response_body then
+ core.response.clear_header_as_body_modified()
Review Comment:
removed this line(```core.response.clear_header_as_body_modified()```) in the new commit
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r859599269
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
If the body is json, and the content-type is text/html, some APP clients may report error, and the terminal user would be effected.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r862619029
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +35,13 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ break_response_content_type = {
Review Comment:
> As for this plugin, I would suggest adding a field to add any response headers instead of just content-type.
Is there any other plugin witch such a field? I'm not sure of which field is suitable for representing any response headers.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on PR #6949:
URL: https://github.com/apache/apisix/pull/6949#issuecomment-1120543729
This PR‘s status is ```Changes approved``` current now. Can i still add more commits to this PR to aim at @spacewander 's suggestion? @tzssangglass .
> As for this plugin, I would suggest adding a field to add any response headers instead of just content-type.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
spacewander commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r868765977
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +37,23 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ headers = {
+ type = "array",
+ items = {
+ type = "object",
+ properties = {
+ key = {
+ type = "string"
Review Comment:
Let's add "minLength = 1" to avoid empty string
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +37,23 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ headers = {
Review Comment:
```suggestion
break_response_headers = {
```
would be better
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +37,23 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ headers = {
+ type = "array",
+ items = {
+ type = "object",
+ properties = {
+ key = {
+ type = "string"
+ },
+ value = {
+ type = "string"
Review Comment:
Ditto
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r868787781
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +37,23 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ headers = {
+ type = "array",
+ items = {
+ type = "object",
+ properties = {
+ key = {
+ type = "string"
Review Comment:
fixed with a new commit
##########
apisix/plugins/api-breaker.lua:
##########
@@ -35,6 +37,23 @@ local schema = {
minimum = 200,
maximum = 599,
},
+ break_response_body = {
+ type = "string"
+ },
+ headers = {
+ type = "array",
+ items = {
+ type = "object",
+ properties = {
+ key = {
+ type = "string"
+ },
+ value = {
+ type = "string"
Review Comment:
fixed with a new commit
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] qihaiyan commented on a diff in pull request #6949: api-breaker plugin returns default response body
Posted by GitBox <gi...@apache.org>.
qihaiyan commented on code in PR #6949:
URL: https://github.com/apache/apisix/pull/6949#discussion_r868713695
##########
apisix/plugins/api-breaker.lua:
##########
@@ -120,7 +132,12 @@ local _M = {
function _M.check_schema(conf)
- return core.schema.check(schema, conf)
+ local ok, err = core.schema.check(schema, conf)
Review Comment:
It's a mistake, no need to change this, i add a new commit to fix 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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org