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/11/17 14:49:17 UTC
[GitHub] [apisix] Firstsawyou opened a new pull request #2776: feat(proxy-cache): the cache_zone field in the schema should be optional
Firstsawyou opened a new pull request #2776:
URL: https://github.com/apache/apisix/pull/2776
fix #2767
### 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. -->
### 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] Firstsawyou commented on a change in pull request #2776: feat(proxy-cache): the cache_zone field in the schema should be optional
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2776:
URL: https://github.com/apache/apisix/pull/2776#discussion_r525260555
##########
File path: apisix/plugins/proxy-cache.lua
##########
@@ -34,7 +34,8 @@ local schema = {
properties = {
cache_zone = {
type = "string",
- minLength = 1
+ minLength = 1,
Review comment:
done.
##########
File path: t/plugin/proxy-cache.t
##########
@@ -94,9 +120,9 @@ __DATA__
}
--- request
GET /t
---- error_code: 400
---- response_body eval
-qr/failed to check the configuration of plugin proxy-cache/
+--- error_code: 200
Review comment:
done.
----------------------------------------------------------------
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] gxthrj commented on a change in pull request #2776: feat(proxy-cache): the cache_zone field in the schema should be optional
Posted by GitBox <gi...@apache.org>.
gxthrj commented on a change in pull request #2776:
URL: https://github.com/apache/apisix/pull/2776#discussion_r525307352
##########
File path: doc/plugins/proxy-cache.md
##########
@@ -31,7 +31,7 @@ The proxy-cache plugin, which provides the ability to cache upstream response da
| Name | Type | Requirement | Default | Valid | Description |
| ------------------ | -------------- | ----------- | ------------------------- | ------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| cache_zone | string | required | | | Specify which cache area to use, each cache area can be configured with different paths. In addition, cache areas can be predefined in conf/config.yaml file |
+| cache_zone | string | optional | disk_cache_one | | Specify which cache area to use, each cache area can be configured with different paths. In addition, cache areas can be predefined in conf/config.yaml file |
Review comment:
We need to tell people the cache will be invalid if `cache_zone` do not match.
----------------------------------------------------------------
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 a change in pull request #2776: feat(proxy-cache): the cache_zone field in the schema should be optional
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #2776:
URL: https://github.com/apache/apisix/pull/2776#discussion_r525263094
##########
File path: doc/zh-cn/plugins/proxy-cache.md
##########
@@ -31,7 +31,7 @@
| 名称 | 类型 | 必选项 | 默认值 | 有效值 | 描述 |
| ------------------ | -------------- | ------ | ------------------------- | ------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------- |
-| cache_zone | string | 必须 | | | 指定使用哪个缓存区域，不同的缓存区域可以配置不同的路径，在conf/config.yaml文件中可以预定义使用的缓存区域 |
+| cache_zone | string | 可选 | disk_cache_one | | 指定使用哪个缓存区域，不同的缓存区域可以配置不同的路径，在conf/config.yaml文件中可以预定义使用的缓存区域 |
Review comment:
Need more details about how to use this config
----------------------------------------------------------------
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] Firstsawyou commented on a change in pull request #2776: feat(proxy-cache): the cache_zone field in the schema should be optional
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2776:
URL: https://github.com/apache/apisix/pull/2776#discussion_r525698125
##########
File path: doc/zh-cn/plugins/proxy-cache.md
##########
@@ -31,7 +31,7 @@
| 名称 | 类型 | 必选项 | 默认值 | 有效值 | 描述 |
| ------------------ | -------------- | ------ | ------------------------- | ------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------- |
-| cache_zone | string | 必须 | | | 指定使用哪个缓存区域，不同的缓存区域可以配置不同的路径，在conf/config.yaml文件中可以预定义使用的缓存区域 |
+| cache_zone | string | 可选 | disk_cache_one | | 指定使用哪个缓存区域，不同的缓存区域可以配置不同的路径，在conf/config.yaml文件中可以预定义使用的缓存区域 |
Review comment:
I will update later.
----------------------------------------------------------------
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 merged pull request #2776: feat(proxy-cache): the cache_zone field in the schema should be optional
Posted by GitBox <gi...@apache.org>.
moonming merged pull request #2776:
URL: https://github.com/apache/apisix/pull/2776
----------------------------------------------------------------
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] Firstsawyou commented on a change in pull request #2776: feat(proxy-cache): the cache_zone field in the schema should be optional
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #2776:
URL: https://github.com/apache/apisix/pull/2776#discussion_r525700820
##########
File path: doc/plugins/proxy-cache.md
##########
@@ -31,7 +31,7 @@ The proxy-cache plugin, which provides the ability to cache upstream response da
| Name | Type | Requirement | Default | Valid | Description |
| ------------------ | -------------- | ----------- | ------------------------- | ------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| cache_zone | string | required | | | Specify which cache area to use, each cache area can be configured with different paths. In addition, cache areas can be predefined in conf/config.yaml file |
+| cache_zone | string | optional | disk_cache_one | | Specify which cache area to use, each cache area can be configured with different paths. In addition, cache areas can be predefined in conf/config.yaml file |
Review comment:
Nice, this sounds great.
----------------------------------------------------------------
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] nic-chen commented on a change in pull request #2776: feat(proxy-cache): the cache_zone field in the schema should be optional
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #2776:
URL: https://github.com/apache/apisix/pull/2776#discussion_r525237312
##########
File path: apisix/plugins/proxy-cache.lua
##########
@@ -34,7 +34,8 @@ local schema = {
properties = {
cache_zone = {
type = "string",
- minLength = 1
+ minLength = 1,
Review comment:
should set a max length too.
##########
File path: t/plugin/proxy-cache.t
##########
@@ -94,9 +120,9 @@ __DATA__
}
--- request
GET /t
---- error_code: 400
---- response_body eval
-qr/failed to check the configuration of plugin proxy-cache/
+--- error_code: 200
Review comment:
we could remove this line when it's 200
----------------------------------------------------------------
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