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/10/19 14:54:23 UTC
[GitHub] [apisix-dashboard] nic-chen opened a new pull request #563: feat: compatible with HTTP status of `admin api`
nic-chen opened a new pull request #563:
URL: https://github.com/apache/apisix-dashboard/pull/563
Please answer these questions before submitting a pull request
- Why submit this pull request?
- [ ] Bug fix
- [x] New feature provided
- [ ] Improve performance
___
### New feature or improvement
----------------------------------------------------------------
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-dashboard] juzhiyuan commented on pull request #563: feat: compatible with HTTP status of `admin api`
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on pull request #563:
URL: https://github.com/apache/apisix-dashboard/pull/563#issuecomment-712559691
Let's merge it now
----------------------------------------------------------------
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-dashboard] ShiningRush commented on a change in pull request #563: feat: compatible with HTTP status of `admin api`
Posted by GitBox <gi...@apache.org>.
ShiningRush commented on a change in pull request #563:
URL: https://github.com/apache/apisix-dashboard/pull/563#discussion_r508170091
##########
File path: api/internal/core/store/validate.go
##########
@@ -125,17 +125,17 @@ func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {
for pluginName, pluginConf := range plugins {
schemaDef := conf.Schema.Get("plugins." + pluginName).String()
if schemaDef == "" {
- return fmt.Errorf("schema not found")
+ return fmt.Errorf("scheme validate failed: schema not found")
}
s, err := gojsonschema.NewSchema(gojsonschema.NewStringLoader(schemaDef))
if err != nil {
- return fmt.Errorf("new schema failed: %w", err)
+ return fmt.Errorf("scheme validate failed: %w", err)
}
ret, err := s.Validate(gojsonschema.NewGoLoader(pluginConf))
if err != nil {
- return fmt.Errorf("validate failed: %w", err)
+ return fmt.Errorf("scheme validate failed: %w", err)
Review comment:
Agree, but we can do it in next version, because there is nobody depend on those message for now. We can release a `rc` version then gradually optimize these code issues
----------------------------------------------------------------
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-dashboard] liuxiran commented on a change in pull request #563: feat: compatible with HTTP status of `admin api`
Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #563:
URL: https://github.com/apache/apisix-dashboard/pull/563#discussion_r508181051
##########
File path: api/internal/core/store/validate.go
##########
@@ -125,17 +125,17 @@ func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {
for pluginName, pluginConf := range plugins {
schemaDef := conf.Schema.Get("plugins." + pluginName).String()
if schemaDef == "" {
- return fmt.Errorf("schema not found")
+ return fmt.Errorf("scheme validate failed: schema not found")
}
s, err := gojsonschema.NewSchema(gojsonschema.NewStringLoader(schemaDef))
if err != nil {
- return fmt.Errorf("new schema failed: %w", err)
+ return fmt.Errorf("scheme validate failed: %w", err)
}
ret, err := s.Validate(gojsonschema.NewGoLoader(pluginConf))
if err != nil {
- return fmt.Errorf("validate failed: %w", err)
+ return fmt.Errorf("scheme validate failed: %w", err)
Review comment:
+1 @ShiningRush
----------------------------------------------------------------
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-dashboard] ShiningRush commented on a change in pull request #563: feat: compatible with HTTP status of `admin api`
Posted by GitBox <gi...@apache.org>.
ShiningRush commented on a change in pull request #563:
URL: https://github.com/apache/apisix-dashboard/pull/563#discussion_r508170091
##########
File path: api/internal/core/store/validate.go
##########
@@ -125,17 +125,17 @@ func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {
for pluginName, pluginConf := range plugins {
schemaDef := conf.Schema.Get("plugins." + pluginName).String()
if schemaDef == "" {
- return fmt.Errorf("schema not found")
+ return fmt.Errorf("scheme validate failed: schema not found")
}
s, err := gojsonschema.NewSchema(gojsonschema.NewStringLoader(schemaDef))
if err != nil {
- return fmt.Errorf("new schema failed: %w", err)
+ return fmt.Errorf("scheme validate failed: %w", err)
}
ret, err := s.Validate(gojsonschema.NewGoLoader(pluginConf))
if err != nil {
- return fmt.Errorf("validate failed: %w", err)
+ return fmt.Errorf("scheme validate failed: %w", err)
Review comment:
Agree, but we can not do it in this version for now. When the first rc version is released, we will optimize it in subsequent versions
----------------------------------------------------------------
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-dashboard] liuxiran commented on a change in pull request #563: feat: compatible with HTTP status of `admin api`
Posted by GitBox <gi...@apache.org>.
liuxiran commented on a change in pull request #563:
URL: https://github.com/apache/apisix-dashboard/pull/563#discussion_r508158329
##########
File path: api/internal/core/store/validate.go
##########
@@ -125,17 +125,17 @@ func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {
for pluginName, pluginConf := range plugins {
schemaDef := conf.Schema.Get("plugins." + pluginName).String()
if schemaDef == "" {
- return fmt.Errorf("schema not found")
+ return fmt.Errorf("scheme validate failed: schema not found")
}
s, err := gojsonschema.NewSchema(gojsonschema.NewStringLoader(schemaDef))
if err != nil {
- return fmt.Errorf("new schema failed: %w", err)
+ return fmt.Errorf("scheme validate failed: %w", err)
}
ret, err := s.Validate(gojsonschema.NewGoLoader(pluginConf))
if err != nil {
- return fmt.Errorf("validate failed: %w", err)
+ return fmt.Errorf("scheme validate failed: %w", err)
Review comment:
I found that the same error messages repeatedly appeared in the files, would it be better to manage error messages in one place just like [error.go](https://github.com/apache/apisix-dashboard/blob/master/api/errno/error.go) before :)?
----------------------------------------------------------------
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-dashboard] ShiningRush commented on a change in pull request #563: feat: compatible with HTTP status of `admin api`
Posted by GitBox <gi...@apache.org>.
ShiningRush commented on a change in pull request #563:
URL: https://github.com/apache/apisix-dashboard/pull/563#discussion_r508170091
##########
File path: api/internal/core/store/validate.go
##########
@@ -125,17 +125,17 @@ func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {
for pluginName, pluginConf := range plugins {
schemaDef := conf.Schema.Get("plugins." + pluginName).String()
if schemaDef == "" {
- return fmt.Errorf("schema not found")
+ return fmt.Errorf("scheme validate failed: schema not found")
}
s, err := gojsonschema.NewSchema(gojsonschema.NewStringLoader(schemaDef))
if err != nil {
- return fmt.Errorf("new schema failed: %w", err)
+ return fmt.Errorf("scheme validate failed: %w", err)
}
ret, err := s.Validate(gojsonschema.NewGoLoader(pluginConf))
if err != nil {
- return fmt.Errorf("validate failed: %w", err)
+ return fmt.Errorf("scheme validate failed: %w", err)
Review comment:
Agree, but we can not do it in this version for now. When the first rc version is released, we will optimize it in next versions
----------------------------------------------------------------
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-dashboard] membphis commented on pull request #563: feat: compatible with HTTP status of `admin api`
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #563:
URL: https://github.com/apache/apisix-dashboard/pull/563#issuecomment-712540623
> No test cases
https://github.com/apache/apisix-dashboard/pull/563/commits/580b88e10edd82d4c02e3d583ce56aff4123f1d1
@moonming you missed them
----------------------------------------------------------------
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-dashboard] juzhiyuan merged pull request #563: feat: compatible with HTTP status of `admin api`
Posted by GitBox <gi...@apache.org>.
juzhiyuan merged pull request #563:
URL: https://github.com/apache/apisix-dashboard/pull/563
----------------------------------------------------------------
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-dashboard] ShiningRush commented on a change in pull request #563: feat: compatible with HTTP status of `admin api`
Posted by GitBox <gi...@apache.org>.
ShiningRush commented on a change in pull request #563:
URL: https://github.com/apache/apisix-dashboard/pull/563#discussion_r508170091
##########
File path: api/internal/core/store/validate.go
##########
@@ -125,17 +125,17 @@ func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {
for pluginName, pluginConf := range plugins {
schemaDef := conf.Schema.Get("plugins." + pluginName).String()
if schemaDef == "" {
- return fmt.Errorf("schema not found")
+ return fmt.Errorf("scheme validate failed: schema not found")
}
s, err := gojsonschema.NewSchema(gojsonschema.NewStringLoader(schemaDef))
if err != nil {
- return fmt.Errorf("new schema failed: %w", err)
+ return fmt.Errorf("scheme validate failed: %w", err)
}
ret, err := s.Validate(gojsonschema.NewGoLoader(pluginConf))
if err != nil {
- return fmt.Errorf("validate failed: %w", err)
+ return fmt.Errorf("scheme validate failed: %w", err)
Review comment:
Agree, but we can not do it in this version for now. When the first `rc` version is released, we will optimize it in next versions.(Because there is nobody depend on those message)
----------------------------------------------------------------
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-dashboard] ShiningRush commented on a change in pull request #563: feat: compatible with HTTP status of `admin api`
Posted by GitBox <gi...@apache.org>.
ShiningRush commented on a change in pull request #563:
URL: https://github.com/apache/apisix-dashboard/pull/563#discussion_r508170091
##########
File path: api/internal/core/store/validate.go
##########
@@ -125,17 +125,17 @@ func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {
for pluginName, pluginConf := range plugins {
schemaDef := conf.Schema.Get("plugins." + pluginName).String()
if schemaDef == "" {
- return fmt.Errorf("schema not found")
+ return fmt.Errorf("scheme validate failed: schema not found")
}
s, err := gojsonschema.NewSchema(gojsonschema.NewStringLoader(schemaDef))
if err != nil {
- return fmt.Errorf("new schema failed: %w", err)
+ return fmt.Errorf("scheme validate failed: %w", err)
}
ret, err := s.Validate(gojsonschema.NewGoLoader(pluginConf))
if err != nil {
- return fmt.Errorf("validate failed: %w", err)
+ return fmt.Errorf("scheme validate failed: %w", err)
Review comment:
Agree, but we can not do it in this version for now. When the first `rc` version is released, we will optimize it in next versions.(Because not there is nobody depend on those message)
----------------------------------------------------------------
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