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/12/09 08:50:51 UTC
[GitHub] [apisix-dashboard] nic-chen opened a new pull request #994: feat: add access log for Manager API
nic-chen opened a new pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994
Please answer these questions before submitting a pull request
- Why submit this pull request?
- [ ] Bugfix
- [x] New feature provided
- [ ] Improve performance
- Related issues
close #603
----------------------------------------------------------------
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] nic-chen commented on a change in pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#discussion_r539973432
##########
File path: api/conf/conf.yaml
##########
@@ -32,6 +32,10 @@ conf:
file_path:
logs/error.log # supports relative path, absolute path, standard output
# such as: logs/error.log, /tmp/logs/error.log, /dev/stdout, /dev/stderr
+ access_log:
+ file_path:
+ logs/access.log # supports relative path, absolute path, standard output
+ # such as: logs/access.log, /tmp/logs/access.log, /dev/stdout, /dev/stderr
Review comment:
updated.
----------------------------------------------------------------
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] codecov-io edited a comment on pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#issuecomment-741638207
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=h1) Report
> Merging [#994](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=desc) (cf6cdcb) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/ffcecfa7909b6a8b6a869fdd63b0e0542aa74bb5?el=desc) (ffcecfa) will **decrease** coverage by `2.23%`.
> The diff coverage is `86.95%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #994 +/- ##
==========================================
- Coverage 44.04% 41.80% -2.24%
==========================================
Files 18 23 +5
Lines 1310 1428 +118
==========================================
+ Hits 577 597 +20
- Misses 642 740 +98
Partials 91 91
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/filter/logging.go](https://codecov.io/gh/apache/apisix-dashboard/pull/994/diff?src=pr&el=tree#diff-YXBpL2ZpbHRlci9sb2dnaW5nLmdv) | `86.95% <86.95%> (ø)` | |
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/994/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `61.29% <0.00%> (-0.40%)` | :arrow_down: |
| [api/filter/authentication.go](https://codecov.io/gh/apache/apisix-dashboard/pull/994/diff?src=pr&el=tree#diff-YXBpL2ZpbHRlci9hdXRoZW50aWNhdGlvbi5nbw==) | `0.00% <0.00%> (ø)` | |
| [api/filter/cors.go](https://codecov.io/gh/apache/apisix-dashboard/pull/994/diff?src=pr&el=tree#diff-YXBpL2ZpbHRlci9jb3JzLmdv) | `0.00% <0.00%> (ø)` | |
| [api/filter/request\_id.go](https://codecov.io/gh/apache/apisix-dashboard/pull/994/diff?src=pr&el=tree#diff-YXBpL2ZpbHRlci9yZXF1ZXN0X2lkLmdv) | `0.00% <0.00%> (ø)` | |
| [api/filter/recover.go](https://codecov.io/gh/apache/apisix-dashboard/pull/994/diff?src=pr&el=tree#diff-YXBpL2ZpbHRlci9yZWNvdmVyLmdv) | `0.00% <0.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=footer). Last update [ffcecfa...cf6cdcb](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] nic-chen commented on a change in pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#discussion_r539973432
##########
File path: api/conf/conf.yaml
##########
@@ -32,6 +32,10 @@ conf:
file_path:
logs/error.log # supports relative path, absolute path, standard output
# such as: logs/error.log, /tmp/logs/error.log, /dev/stdout, /dev/stderr
+ access_log:
+ file_path:
+ logs/access.log # supports relative path, absolute path, standard output
+ # such as: logs/access.log, /tmp/logs/access.log, /dev/stdout, /dev/stderr
Review comment:
update
----------------------------------------------------------------
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 #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#issuecomment-742497940
> I think you need to add unit tests.
I think we should use E2E test here. @nic-chen please confirm
----------------------------------------------------------------
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] nic-chen commented on a change in pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#discussion_r540286871
##########
File path: api/filter/logging_test.go
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package filter
+
+import (
+ "net/http"
+ "net/http/httptest"
+ "testing"
+
+ "github.com/gin-gonic/gin"
+ "github.com/stretchr/testify/assert"
+
+ "github.com/apisix/manager-api/log"
+)
+
+func performRequest(r http.Handler, method, path string) *httptest.ResponseRecorder {
+ req := httptest.NewRequest(method, path, nil)
+ w := httptest.NewRecorder()
+ r.ServeHTTP(w, req)
+ return w
+}
+
+func TestRequestLogHandler(t *testing.T) {
+ r := gin.New()
+ logger := log.GetLogger(log.AccessLog)
+ r.Use(RequestLogHandler(logger))
+ r.GET("/", func(c *gin.Context) {
+ })
+
+ w := performRequest(r, "GET", "/")
+ assert.Equal(t, 200, w.Code)
+}
Review comment:
fixed.
----------------------------------------------------------------
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] codecov-io commented on pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#issuecomment-741638207
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=h1) Report
> Merging [#994](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=desc) (1629a2e) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/ffcecfa7909b6a8b6a869fdd63b0e0542aa74bb5?el=desc) (ffcecfa) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #994 +/- ##
=======================================
Coverage 44.04% 44.04%
=======================================
Files 18 18
Lines 1310 1310
=======================================
Hits 577 577
Misses 642 642
Partials 91 91
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=footer). Last update [ffcecfa...1629a2e](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] nic-chen commented on a change in pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#discussion_r540164164
##########
File path: api/conf/conf.go
##########
@@ -149,6 +156,17 @@ func setConf() {
}
}
+ // access log
+ if config.Conf.Log.AccessLog.FilePath != "" {
+ AccessLogPath = config.Conf.Log.AccessLog.FilePath
+ }
+ if !filepath.IsAbs(AccessLogPath) {
+ AccessLogPath, err = filepath.Abs(WorkDir + "/" + AccessLogPath)
Review comment:
fixed.
----------------------------------------------------------------
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 a change in pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#discussion_r539232412
##########
File path: api/conf/conf.yaml
##########
@@ -32,6 +32,10 @@ conf:
file_path:
logs/error.log # supports relative path, absolute path, standard output
# such as: logs/error.log, /tmp/logs/error.log, /dev/stdout, /dev/stderr
+ access_log:
+ file_path:
+ logs/access.log # supports relative path, absolute path, standard output
+ # such as: logs/access.log, /tmp/logs/access.log, /dev/stdout, /dev/stderr
Review comment:
print one example, show the format
----------------------------------------------------------------
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 a change in pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#discussion_r540226767
##########
File path: api/filter/logging_test.go
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package filter
+
+import (
+ "net/http"
+ "net/http/httptest"
+ "testing"
+
+ "github.com/gin-gonic/gin"
+ "github.com/stretchr/testify/assert"
+
+ "github.com/apisix/manager-api/log"
+)
+
+func performRequest(r http.Handler, method, path string) *httptest.ResponseRecorder {
+ req := httptest.NewRequest(method, path, nil)
+ w := httptest.NewRecorder()
+ r.ServeHTTP(w, req)
+ return w
+}
+
+func TestRequestLogHandler(t *testing.T) {
+ r := gin.New()
+ logger := log.GetLogger(log.AccessLog)
+ r.Use(RequestLogHandler(logger))
+ r.GET("/", func(c *gin.Context) {
+ })
+
+ w := performRequest(r, "GET", "/")
+ assert.Equal(t, 200, w.Code)
+}
Review comment:
please check your code editor setting, missing blank line at the end of file
----------------------------------------------------------------
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] nic-chen commented on a change in pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#discussion_r540164303
##########
File path: api/conf/conf.go
##########
@@ -69,8 +70,14 @@ type ErrorLog struct {
FilePath string `yaml:"file_path"`
}
+type AccessLog struct {
+ Level string
Review comment:
fixed.
----------------------------------------------------------------
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] tokers commented on a change in pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#discussion_r540011482
##########
File path: api/conf/conf.go
##########
@@ -149,6 +156,17 @@ func setConf() {
}
}
+ // access log
+ if config.Conf.Log.AccessLog.FilePath != "" {
+ AccessLogPath = config.Conf.Log.AccessLog.FilePath
+ }
+ if !filepath.IsAbs(AccessLogPath) {
+ AccessLogPath, err = filepath.Abs(WorkDir + "/" + AccessLogPath)
Review comment:
Use `filepath.Join` instead of concatenating them manually.
##########
File path: api/filter/logging.go
##########
@@ -16,4 +16,53 @@
*/
package filter
-//for logging access log, will refactor it in a new pr.
+import (
+ "bytes"
+ "time"
+
+ "github.com/gin-gonic/gin"
+ "go.uber.org/zap"
+)
+
+func RequestLogHandler(logger *zap.SugaredLogger) gin.HandlerFunc {
+ return func(c *gin.Context) {
+ start, host, remoteIP, path, method := time.Now(), c.Request.Host, c.ClientIP(), c.Request.URL.Path, c.Request.Method
+ query := c.Request.URL.RawQuery
+ requestId := c.Writer.Header().Get("X-Request-Id")
+
+ blw := &bodyLogWriter{body: bytes.NewBufferString(""), ResponseWriter: c.Writer}
Review comment:
Why use `bytes.NewBufferString` but the parameter is an empty string, just use `bytes.NewBuffer` is enough.
##########
File path: api/conf/conf.go
##########
@@ -69,8 +70,14 @@ type ErrorLog struct {
FilePath string `yaml:"file_path"`
}
+type AccessLog struct {
+ Level string
Review comment:
log level in access log is no sense.
##########
File path: api/conf/conf.go
##########
@@ -69,8 +70,14 @@ type ErrorLog struct {
FilePath string `yaml:"file_path"`
}
+type AccessLog struct {
+ Level string
Review comment:
You can change the zapcore encoder to remove the level encoding.
----------------------------------------------------------------
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] codecov-io edited a comment on pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#issuecomment-741638207
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=h1) Report
> Merging [#994](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=desc) (45129f8) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/ffcecfa7909b6a8b6a869fdd63b0e0542aa74bb5?el=desc) (ffcecfa) will **decrease** coverage by `2.23%`.
> The diff coverage is `86.95%`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #994 +/- ##
==========================================
- Coverage 44.04% 41.80% -2.24%
==========================================
Files 18 23 +5
Lines 1310 1428 +118
==========================================
+ Hits 577 597 +20
- Misses 642 740 +98
Partials 91 91
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/filter/logging.go](https://codecov.io/gh/apache/apisix-dashboard/pull/994/diff?src=pr&el=tree#diff-YXBpL2ZpbHRlci9sb2dnaW5nLmdv) | `86.95% <86.95%> (ø)` | |
| [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/994/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `61.29% <0.00%> (-0.40%)` | :arrow_down: |
| [api/filter/authentication.go](https://codecov.io/gh/apache/apisix-dashboard/pull/994/diff?src=pr&el=tree#diff-YXBpL2ZpbHRlci9hdXRoZW50aWNhdGlvbi5nbw==) | `0.00% <0.00%> (ø)` | |
| [api/filter/recover.go](https://codecov.io/gh/apache/apisix-dashboard/pull/994/diff?src=pr&el=tree#diff-YXBpL2ZpbHRlci9yZWNvdmVyLmdv) | `0.00% <0.00%> (ø)` | |
| [api/filter/request\_id.go](https://codecov.io/gh/apache/apisix-dashboard/pull/994/diff?src=pr&el=tree#diff-YXBpL2ZpbHRlci9yZXF1ZXN0X2lkLmdv) | `0.00% <0.00%> (ø)` | |
| [api/filter/cors.go](https://codecov.io/gh/apache/apisix-dashboard/pull/994/diff?src=pr&el=tree#diff-YXBpL2ZpbHRlci9jb3JzLmdv) | `0.00% <0.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=footer). Last update [ffcecfa...45129f8](https://codecov.io/gh/apache/apisix-dashboard/pull/994?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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] moonming commented on pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#issuecomment-742383449
I think you need to add unit tests.
----------------------------------------------------------------
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] nic-chen commented on a change in pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#discussion_r540164892
##########
File path: api/filter/logging.go
##########
@@ -16,4 +16,53 @@
*/
package filter
-//for logging access log, will refactor it in a new pr.
+import (
+ "bytes"
+ "time"
+
+ "github.com/gin-gonic/gin"
+ "go.uber.org/zap"
+)
+
+func RequestLogHandler(logger *zap.SugaredLogger) gin.HandlerFunc {
+ return func(c *gin.Context) {
+ start, host, remoteIP, path, method := time.Now(), c.Request.Host, c.ClientIP(), c.Request.URL.Path, c.Request.Method
+ query := c.Request.URL.RawQuery
+ requestId := c.Writer.Header().Get("X-Request-Id")
+
+ blw := &bodyLogWriter{body: bytes.NewBufferString(""), ResponseWriter: c.Writer}
Review comment:
fixed.
----------------------------------------------------------------
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] nic-chen edited a comment on pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
nic-chen edited a comment on pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#issuecomment-742517813
> > I think you need to add unit tests.
>
> I think we should use E2E test here. @nic-chen please confirm
This access log is for Manager API, does not involve DP, so e2e test should not be needed.
I have added a simple unit test case to confirm that it will not cause errors.
cc @membphis @moonming
----------------------------------------------------------------
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] nic-chen commented on pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#issuecomment-742517813
> > I think you need to add unit tests.
>
> I think we should use E2E test here. @nic-chen please confirm
This access log is for Manager API, does not involve DP, so e2e test should not be needed.
I have added a simple unit test case to confirm that it will not cause errors.
----------------------------------------------------------------
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] nic-chen edited a comment on pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
nic-chen edited a comment on pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994#issuecomment-742517813
> > I think you need to add unit tests.
>
> I think we should use E2E test here. @nic-chen please confirm
This access log is for Manager API, does not involve DP, so e2e test should not be needed, cli test is more suitable (added before).
I have added a simple unit test case to confirm that it will not cause errors.
cc @membphis @moonming
----------------------------------------------------------------
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] tokers merged pull request #994: feat: add access log for Manager API
Posted by GitBox <gi...@apache.org>.
tokers merged pull request #994:
URL: https://github.com/apache/apisix-dashboard/pull/994
----------------------------------------------------------------
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