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/02 15:24:37 UTC

[GitHub] [apisix-dashboard] starsz opened a new pull request #958: feat: implate API to get apisix instances status

starsz opened a new pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958


   Please answer these questions before submitting a pull request
   
   - Why submit this pull request?
   - [ ] Bugfix
   - [x] New feature provided
   - [ ] Improve performance
   
   - Related issues
   
   fix #849 
   ___
   ### New feature or improvement
   - Describe the details and related test reports.
   
   1. Implate API( `/apisix/server_info/:id` ) to get APISIX instances status.
   2. Add e2e test and unit test.
   


----------------------------------------------------------------
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 #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#issuecomment-747462413


   @starsz CI failed ^_^


----------------------------------------------------------------
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] starsz commented on a change in pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r544859611



##########
File path: api/test/e2e/server_info_test.go
##########
@@ -0,0 +1,147 @@
+/*
+ * 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 e2e
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"math/rand"
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+const (
+	ServerInfoPrefix = "/apisix/data_plane/server_info"
+)
+
+var (
+	s = &EtcdV3Storage{}
+)
+
+type ServerInfo struct {
+	ID             string `json:"id"`
+	LastReportTime int64  `json:"last_report_time,omitempty"`
+	UpTime         int64  `json:"up_time,omitempty"`
+	BootTime       int64  `json:"boot_time,omitempty"`
+	EtcdVersion    string `json:"etcd_version,omitempty"`
+	Hostname       string `json:"hostname,omitempty"`
+	Version        string `json:"version,omitempty"`
+}
+
+func putServerInfo(id, val string) error {
+	ctx, _ := context.WithTimeout(context.Background(), 2*time.Second)
+	key := fmt.Sprintf("%s/%s", ServerInfoPrefix, id)
+	return s.Create(ctx, key, val)
+}
+
+func deleteServerInfo(ids []string) error {
+	ctx, _ := context.WithTimeout(context.Background(), 2*time.Second)
+
+	var keys []string
+	for _, id := range ids {
+		keys = append(keys, fmt.Sprintf("%s/%s", ServerInfoPrefix, id))
+	}
+
+	return s.BatchDelete(ctx, keys)
+}
+
+func genServerInfo(id, hostname string) string {

Review comment:
       OK.Let me 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-dashboard] nic-chen edited a comment on pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
nic-chen edited a comment on pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#issuecomment-747842210


   @starsz  Manager API start failed in CI.


----------------------------------------------------------------
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] starsz commented on pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
starsz commented on pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#issuecomment-747873968


   Yes,need fix first.
   
   https://github.com/apache/apisix/pull/3075


----------------------------------------------------------------
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 #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r545505526



##########
File path: api/test/docker/docker-compose.yaml
##########
@@ -149,14 +150,15 @@ services:
         ipv4_address: 172.16.238.30
 
   apisix2:
+    hostname: apisix_server2
     build:
       context: ../../
       dockerfile: test/docker/Dockerfile-apisix
       args:
-        - APISIX_VERSION=2.1
+        - APISIX_VERSION=master
     restart: always
     volumes:
-      - ./apisix_config.yaml:/usr/local/apisix/conf/config.yaml:ro
+      - ./apisix_config2.yaml:/usr/local/apisix/conf/config.yaml:ro

Review comment:
       Why using `apisix_config2`?




----------------------------------------------------------------
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] starsz commented on pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
starsz commented on pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#issuecomment-747426753


   > @starsz what happen to your branch..
   
   I had done some wrong operation on my git branch. But luckily, it looks normal 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] nic-chen commented on a change in pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r545603109



##########
File path: api/test/docker/apisix_config2.yaml
##########
@@ -93,4 +93,5 @@ plugin_attr:
 
   server-info:
     report_interval: 10
-    report_ttl: 3600
\ No newline at end of file
+    report_ttl: 3600
+

Review comment:
       `plugins` can't be removed, because we have e2e test cases for skywalking which is not enable by default. 




----------------------------------------------------------------
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 #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r542085785



##########
File path: api/test/e2e/server_info_test.go
##########
@@ -0,0 +1,147 @@
+/*
+ * 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 e2e
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"math/rand"
+	"net/http"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+const (
+	ServerInfoPrefix = "/apisix/data_plane/server_info"
+)
+
+var (
+	s = &EtcdV3Storage{}
+)
+
+type ServerInfo struct {
+	ID             string `json:"id"`
+	LastReportTime int64  `json:"last_report_time,omitempty"`
+	UpTime         int64  `json:"up_time,omitempty"`
+	BootTime       int64  `json:"boot_time,omitempty"`
+	EtcdVersion    string `json:"etcd_version,omitempty"`
+	Hostname       string `json:"hostname,omitempty"`
+	Version        string `json:"version,omitempty"`
+}
+
+func putServerInfo(id, val string) error {
+	ctx, _ := context.WithTimeout(context.Background(), 2*time.Second)
+	key := fmt.Sprintf("%s/%s", ServerInfoPrefix, id)
+	return s.Create(ctx, key, val)
+}
+
+func deleteServerInfo(ids []string) error {
+	ctx, _ := context.WithTimeout(context.Background(), 2*time.Second)
+
+	var keys []string
+	for _, id := range ids {
+		keys = append(keys, fmt.Sprintf("%s/%s", ServerInfoPrefix, id))
+	}
+
+	return s.BatchDelete(ctx, keys)
+}
+
+func genServerInfo(id, hostname string) string {

Review comment:
       We should let APISIX generate server info instead of directly operating ETCD to simulate.
   




----------------------------------------------------------------
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 #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r535011824



##########
File path: api/internal/core/entity/entity.go
##########
@@ -220,3 +220,12 @@ type Script struct {
 	ID     string      `json:"id"`
 	Script interface{} `json:"script,omitempty"`
 }
+
+type ServerInfo struct {
+	BaseInfo
+	LastReportTime int    `json:"last_report_time,omitempty"`
+	UpTime         int    `json:"up_time,omitempty"`

Review comment:
       Ditto.

##########
File path: api/internal/core/entity/entity.go
##########
@@ -220,3 +220,12 @@ type Script struct {
 	ID     string      `json:"id"`
 	Script interface{} `json:"script,omitempty"`
 }
+
+type ServerInfo struct {
+	BaseInfo
+	LastReportTime int    `json:"last_report_time,omitempty"`

Review comment:
       By convention we use `int64` to store timestamp.




----------------------------------------------------------------
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] starsz commented on a change in pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r535896438



##########
File path: api/internal/core/entity/entity.go
##########
@@ -220,3 +220,12 @@ type Script struct {
 	ID     string      `json:"id"`
 	Script interface{} `json:"script,omitempty"`
 }
+
+type ServerInfo struct {
+	BaseInfo
+	LastReportTime int    `json:"last_report_time,omitempty"`

Review comment:
       Fixed.

##########
File path: api/internal/core/entity/entity.go
##########
@@ -220,3 +220,12 @@ type Script struct {
 	ID     string      `json:"id"`
 	Script interface{} `json:"script,omitempty"`
 }
+
+type ServerInfo struct {
+	BaseInfo
+	LastReportTime int    `json:"last_report_time,omitempty"`
+	UpTime         int    `json:"up_time,omitempty"`

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 edited a comment on pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#issuecomment-747498384


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=h1) Report
   > Merging [#958](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=desc) (68d8fee) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/92fad09da6247c1a8b7010505bd9039acf8dc39f?el=desc) (92fad09) will **decrease** coverage by `0.00%`.
   > The diff coverage is `40.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/958/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #958      +/-   ##
   ==========================================
   - Coverage   40.13%   40.13%   -0.01%     
   ==========================================
     Files          26       27       +1     
     Lines        1572     1602      +30     
   ==========================================
   + Hits          631      643      +12     
   - Misses        850      867      +17     
   - Partials       91       92       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `0.00% <ø> (ø)` | |
   | [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (ø)` | |
   | [api/internal/handler/server\_info/server\_info.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmVyX2luZm8vc2VydmVyX2luZm8uZ28=) | `57.14% <57.14%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/958?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/958?src=pr&el=footer). Last update [92fad09...68d8fee](https://codecov.io/gh/apache/apisix-dashboard/pull/958?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] starsz commented on pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
starsz commented on pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#issuecomment-747843375


   Yeah. I found a bug in apisix server_info that caused this problem.
   So I need to fix apisix first.


----------------------------------------------------------------
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] starsz commented on a change in pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r545604401



##########
File path: api/test/docker/apisix_config2.yaml
##########
@@ -93,4 +93,5 @@ plugin_attr:
 
   server-info:
     report_interval: 10
-    report_ttl: 3600
\ No newline at end of file
+    report_ttl: 3600
+

Review comment:
       I think it's not the duty of this PR. Maybe we need another PR 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-dashboard] starsz commented on a change in pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r544866618



##########
File path: api/internal/handler/server_info/server_info_test.go
##########
@@ -0,0 +1,156 @@
+/*
+ * 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 server_info
+
+import (
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/shiningrush/droplet"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/apisix/manager-api/internal/core/entity"
+	"github.com/apisix/manager-api/internal/core/storage"
+	"github.com/apisix/manager-api/internal/core/store"
+)
+
+const (
+	SleepTime = 100 * time.Millisecond
+)
+
+var serverInfoHandler *Handler
+
+func init() {
+	err := storage.InitETCDClient([]string{"127.0.0.1:2379"})
+
+	if err != nil {
+		panic(err)
+	}
+	err = store.InitStores()
+	if err != nil {
+		panic(err)
+	}
+
+	serverInfoHandler = &Handler{
+		serverInfoStore: store.GetStore(store.HubKeyServerInfo),
+	}
+}
+
+func create(t *testing.T, ctx droplet.Context, info *entity.ServerInfo) {

Review comment:
       Got 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#issuecomment-747498384


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=h1) Report
   > Merging [#958](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=desc) (f52a4d9) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/92fad09da6247c1a8b7010505bd9039acf8dc39f?el=desc) (92fad09) will **increase** coverage by `0.36%`.
   > The diff coverage is `40.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/958/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #958      +/-   ##
   ==========================================
   + Coverage   40.13%   40.50%   +0.36%     
   ==========================================
     Files          26       27       +1     
     Lines        1572     1627      +55     
   ==========================================
   + Hits          631      659      +28     
   - Misses        850      873      +23     
   - Partials       91       95       +4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `0.00% <ø> (ø)` | |
   | [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (ø)` | |
   | [api/internal/handler/server\_info/server\_info.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmVyX2luZm8vc2VydmVyX2luZm8uZ28=) | `57.14% <57.14%> (ø)` | |
   | [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `46.59% <0.00%> (+0.03%)` | :arrow_up: |
   | [api/internal/utils/utils.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL3V0aWxzLmdv) | `56.89% <0.00%> (+5.46%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/958?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/958?src=pr&el=footer). Last update [92fad09...f52a4d9](https://codecov.io/gh/apache/apisix-dashboard/pull/958?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 pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#issuecomment-747419859


   @starsz  what happen to your branch.. 


----------------------------------------------------------------
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 #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#issuecomment-747842210


   @starsz  Manager API start failed.


----------------------------------------------------------------
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 #958: feat: implement API to get apisix instances status

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



##########
File path: api/test/docker/apisix_config2.yaml
##########
@@ -93,4 +93,5 @@ plugin_attr:
 
   server-info:
     report_interval: 10
-    report_ttl: 3600
\ No newline at end of file
+    report_ttl: 3600
+

Review comment:
       we can remove `apisix.ssl. ssl_cert` and `apisix.ssl. ssl_cert_key`

##########
File path: api/test/docker/apisix_config2.yaml
##########
@@ -93,4 +93,5 @@ plugin_attr:
 
   server-info:
     report_interval: 10
-    report_ttl: 3600
\ No newline at end of file
+    report_ttl: 3600
+

Review comment:
       same issue with file `api/test/docker/apisix_config.yaml`

##########
File path: api/test/docker/apisix_config2.yaml
##########
@@ -93,4 +93,5 @@ plugin_attr:
 
   server-info:
     report_interval: 10
-    report_ttl: 3600
\ No newline at end of file
+    report_ttl: 3600
+

Review comment:
       and we can remove `plugins` too




----------------------------------------------------------------
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 #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#issuecomment-747498384


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=h1) Report
   > Merging [#958](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=desc) (57c4ac6) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/92fad09da6247c1a8b7010505bd9039acf8dc39f?el=desc) (92fad09) will **decrease** coverage by `0.00%`.
   > The diff coverage is `40.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/958/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #958      +/-   ##
   ==========================================
   - Coverage   40.13%   40.13%   -0.01%     
   ==========================================
     Files          26       27       +1     
     Lines        1572     1602      +30     
   ==========================================
   + Hits          631      643      +12     
   - Misses        850      867      +17     
   - Partials       91       92       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `0.00% <ø> (ø)` | |
   | [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (ø)` | |
   | [api/internal/handler/server\_info/server\_info.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmVyX2luZm8vc2VydmVyX2luZm8uZ28=) | `57.14% <57.14%> (ø)` | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/958?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/958?src=pr&el=footer). Last update [92fad09...57c4ac6](https://codecov.io/gh/apache/apisix-dashboard/pull/958?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] tokers merged pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
tokers merged pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958


   


----------------------------------------------------------------
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] starsz commented on pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
starsz commented on pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#issuecomment-747970390


   Ping @membphis @tokers @nic-chen Hi, I think it's ok 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] membphis commented on pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#issuecomment-747861029


   CI failed:
   
   ```
   go: finding github.com/valyala/bytebufferpool v1.0.0
   go: finding github.com/klauspost/cpuid v1.2.1
   panic: Post http://127.0.0.1:9000/apisix/admin/user/login: dial tcp 127.0.0.1:9000: connect: connection refused
   
   goroutine 1 [running]:
   e2e.init.0()
   	/home/runner/work/apisix-dashboard/apisix-dashboard/api/test/e2e/base.go:51 +0x3e6
   exit status 2
   FAIL	e2e	0.008s
   Error: Process completed with exit code 1.


----------------------------------------------------------------
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] starsz commented on a change in pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r545600858



##########
File path: api/test/docker/docker-compose.yaml
##########
@@ -149,14 +150,15 @@ services:
         ipv4_address: 172.16.238.30
 
   apisix2:
+    hostname: apisix_server2
     build:
       context: ../../
       dockerfile: test/docker/Dockerfile-apisix
       args:
         - APISIX_VERSION=2.1

Review comment:
       OK




----------------------------------------------------------------
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 #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r542085107



##########
File path: api/internal/handler/server_info/server_info_test.go
##########
@@ -0,0 +1,156 @@
+/*
+ * 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 server_info
+
+import (
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/shiningrush/droplet"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/apisix/manager-api/internal/core/entity"
+	"github.com/apisix/manager-api/internal/core/storage"
+	"github.com/apisix/manager-api/internal/core/store"
+)
+
+const (
+	SleepTime = 100 * time.Millisecond
+)
+
+var serverInfoHandler *Handler
+
+func init() {
+	err := storage.InitETCDClient([]string{"127.0.0.1:2379"})
+
+	if err != nil {
+		panic(err)
+	}
+	err = store.InitStores()
+	if err != nil {
+		panic(err)
+	}
+
+	serverInfoHandler = &Handler{
+		serverInfoStore: store.GetStore(store.HubKeyServerInfo),
+	}
+}
+
+func create(t *testing.T, ctx droplet.Context, info *entity.ServerInfo) {

Review comment:
       @starsz  we have started to refactor the unit test cases. you can refer : https://github.com/apache/apisix-dashboard/blob/master/api/internal/handler/consumer/consumer_test.go




----------------------------------------------------------------
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] starsz commented on a change in pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r537194030



##########
File path: api/internal/core/store/storehub.go
##########
@@ -144,5 +145,17 @@ func InitStores() error {
 		return err
 	}
 
+	err = InitStore(HubKeyServerInfo, GenericStoreOption{
+		BasePath: "/apisix/data_plane/server_info",
+		ObjType:  reflect.TypeOf(entity.ServerInfo{}),
+		KeyFunc: func(obj interface{}) string {

Review comment:
       I have tried to fix it, but it will be recovery by `goland`.
   So I think it's the behavior of `goland`.




----------------------------------------------------------------
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] starsz commented on a change in pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r537195023



##########
File path: api/internal/handler/server_info/server_info_test.go
##########
@@ -0,0 +1,156 @@
+/*
+ * 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 server_info
+
+import (
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/shiningrush/droplet"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/apisix/manager-api/internal/core/entity"
+	"github.com/apisix/manager-api/internal/core/storage"
+	"github.com/apisix/manager-api/internal/core/store"
+)
+
+const (
+	SleepTime = 100 * time.Millisecond
+)
+
+var serverInfoHandler *Handler
+
+func init() {
+	err := storage.InitETCDClient([]string{"127.0.0.1:2379"})
+
+	if err != nil {
+		panic(err)
+	}
+	err = store.InitStores()
+	if err != nil {
+		panic(err)
+	}
+
+	serverInfoHandler = &Handler{
+		serverInfoStore: store.GetStore(store.HubKeyServerInfo),
+	}
+}
+
+func create(t *testing.T, ctx droplet.Context, info *entity.ServerInfo) {

Review comment:
       Many unit tests depend on a real etcd cluster.
   I think maybe we need to create a TODO to refactor those test cases.

##########
File path: api/internal/handler/server_info/server_info_test.go
##########
@@ -0,0 +1,156 @@
+/*
+ * 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 server_info
+
+import (
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/shiningrush/droplet"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/apisix/manager-api/internal/core/entity"
+	"github.com/apisix/manager-api/internal/core/storage"
+	"github.com/apisix/manager-api/internal/core/store"
+)
+
+const (
+	SleepTime = 100 * time.Millisecond
+)
+
+var serverInfoHandler *Handler
+
+func init() {
+	err := storage.InitETCDClient([]string{"127.0.0.1:2379"})
+
+	if err != nil {
+		panic(err)
+	}
+	err = store.InitStores()
+	if err != nil {
+		panic(err)
+	}
+
+	serverInfoHandler = &Handler{
+		serverInfoStore: store.GetStore(store.HubKeyServerInfo),
+	}
+}
+
+func create(t *testing.T, ctx droplet.Context, info *entity.ServerInfo) {
+	err := serverInfoHandler.serverInfoStore.Create(ctx.Context(), info)
+	assert.Nil(t, err)
+}
+
+func delete(t *testing.T, ctx droplet.Context, ids []string) {
+	err := serverInfoHandler.serverInfoStore.BatchDelete(ctx.Context(), ids)
+	assert.Nil(t, err)
+}
+
+func testGet(t *testing.T, ctx droplet.Context, id string) {
+	input := &GetInput{ID: id}
+
+	ctx.SetInput(input)
+	res, err := serverInfoHandler.Get(ctx)
+	assert.Nil(t, err)
+
+	stored := res.(*entity.ServerInfo)
+	assert.Equal(t, stored.ID, input.ID)
+}
+
+func testList(t *testing.T, ctx droplet.Context, ids []string, hostname string) {
+	input := &ListInput{Hostname: hostname, Pagination: store.Pagination{PageSize: 10, PageNumber: 1}}
+
+	ctx.SetInput(input)
+	res, err := serverInfoHandler.List(ctx)
+	assert.Nil(t, err)
+
+	data := res.(*store.ListOutput)
+	assert.Equal(t, len(data.Rows), len(ids))
+
+	var find bool
+	for _, id := range ids {
+		find = false
+		for _, row := range data.Rows {
+			si := row.(*entity.ServerInfo)
+			if si.ID == id {
+

Review comment:
       OK, 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 commented on a change in pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
nic-chen commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r545140026



##########
File path: api/test/docker/docker-compose.yaml
##########
@@ -149,14 +150,15 @@ services:
         ipv4_address: 172.16.238.30
 
   apisix2:
+    hostname: apisix_server2
     build:
       context: ../../
       dockerfile: test/docker/Dockerfile-apisix
       args:
         - APISIX_VERSION=2.1

Review comment:
       we should use `master` as APISIX_VERSION here.
   `server-info` is new plugin that not in APISIX 2.1.
   




----------------------------------------------------------------
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 #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r536140077



##########
File path: api/internal/core/store/storehub.go
##########
@@ -144,5 +145,17 @@ func InitStores() error {
 		return err
 	}
 
+	err = InitStore(HubKeyServerInfo, GenericStoreOption{
+		BasePath: "/apisix/data_plane/server_info",
+		ObjType:  reflect.TypeOf(entity.ServerInfo{}),
+		KeyFunc: func(obj interface{}) string {

Review comment:
       Style: should keep the indent same when initializing the structure.

##########
File path: api/internal/handler/server_info/server_info_test.go
##########
@@ -0,0 +1,156 @@
+/*
+ * 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 server_info
+
+import (
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/shiningrush/droplet"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/apisix/manager-api/internal/core/entity"
+	"github.com/apisix/manager-api/internal/core/storage"
+	"github.com/apisix/manager-api/internal/core/store"
+)
+
+const (
+	SleepTime = 100 * time.Millisecond
+)
+
+var serverInfoHandler *Handler
+
+func init() {
+	err := storage.InitETCDClient([]string{"127.0.0.1:2379"})
+
+	if err != nil {
+		panic(err)
+	}
+	err = store.InitStores()
+	if err != nil {
+		panic(err)
+	}
+
+	serverInfoHandler = &Handler{
+		serverInfoStore: store.GetStore(store.HubKeyServerInfo),
+	}
+}
+
+func create(t *testing.T, ctx droplet.Context, info *entity.ServerInfo) {
+	err := serverInfoHandler.serverInfoStore.Create(ctx.Context(), info)
+	assert.Nil(t, err)
+}
+
+func delete(t *testing.T, ctx droplet.Context, ids []string) {
+	err := serverInfoHandler.serverInfoStore.BatchDelete(ctx.Context(), ids)
+	assert.Nil(t, err)
+}
+
+func testGet(t *testing.T, ctx droplet.Context, id string) {
+	input := &GetInput{ID: id}
+
+	ctx.SetInput(input)
+	res, err := serverInfoHandler.Get(ctx)
+	assert.Nil(t, err)
+
+	stored := res.(*entity.ServerInfo)
+	assert.Equal(t, stored.ID, input.ID)
+}
+
+func testList(t *testing.T, ctx droplet.Context, ids []string, hostname string) {
+	input := &ListInput{Hostname: hostname, Pagination: store.Pagination{PageSize: 10, PageNumber: 1}}
+
+	ctx.SetInput(input)
+	res, err := serverInfoHandler.List(ctx)
+	assert.Nil(t, err)
+
+	data := res.(*store.ListOutput)
+	assert.Equal(t, len(data.Rows), len(ids))
+
+	var find bool
+	for _, id := range ids {
+		find = false
+		for _, row := range data.Rows {
+			si := row.(*entity.ServerInfo)
+			if si.ID == id {
+

Review comment:
       Style: redundant empty line.

##########
File path: api/internal/handler/server_info/server_info_test.go
##########
@@ -0,0 +1,156 @@
+/*
+ * 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 server_info
+
+import (
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/shiningrush/droplet"
+	"github.com/stretchr/testify/assert"
+
+	"github.com/apisix/manager-api/internal/core/entity"
+	"github.com/apisix/manager-api/internal/core/storage"
+	"github.com/apisix/manager-api/internal/core/store"
+)
+
+const (
+	SleepTime = 100 * time.Millisecond
+)
+
+var serverInfoHandler *Handler
+
+func init() {
+	err := storage.InitETCDClient([]string{"127.0.0.1:2379"})
+
+	if err != nil {
+		panic(err)
+	}
+	err = store.InitStores()
+	if err != nil {
+		panic(err)
+	}
+
+	serverInfoHandler = &Handler{
+		serverInfoStore: store.GetStore(store.HubKeyServerInfo),
+	}
+}
+
+func create(t *testing.T, ctx droplet.Context, info *entity.ServerInfo) {

Review comment:
       Shouldn't we use a mocked store here? Rather than depending on a real etcd cluster.




----------------------------------------------------------------
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 #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#issuecomment-747498384


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=h1) Report
   > Merging [#958](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=desc) (616f68b) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/92fad09da6247c1a8b7010505bd9039acf8dc39f?el=desc) (92fad09) will **increase** coverage by `0.36%`.
   > The diff coverage is `40.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/958/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #958      +/-   ##
   ==========================================
   + Coverage   40.13%   40.50%   +0.36%     
   ==========================================
     Files          26       27       +1     
     Lines        1572     1627      +55     
   ==========================================
   + Hits          631      659      +28     
   - Misses        850      873      +23     
   - Partials       91       95       +4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/958?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `0.00% <ø> (ø)` | |
   | [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (ø)` | |
   | [api/internal/handler/server\_info/server\_info.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmVyX2luZm8vc2VydmVyX2luZm8uZ28=) | `57.14% <57.14%> (ø)` | |
   | [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `46.59% <0.00%> (+0.03%)` | :arrow_up: |
   | [api/internal/utils/utils.go](https://codecov.io/gh/apache/apisix-dashboard/pull/958/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL3V0aWxzLmdv) | `56.89% <0.00%> (+5.46%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/958?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/958?src=pr&el=footer). Last update [92fad09...616f68b](https://codecov.io/gh/apache/apisix-dashboard/pull/958?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] starsz commented on a change in pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r545536735



##########
File path: api/test/docker/docker-compose.yaml
##########
@@ -149,14 +150,15 @@ services:
         ipv4_address: 172.16.238.30
 
   apisix2:
+    hostname: apisix_server2
     build:
       context: ../../
       dockerfile: test/docker/Dockerfile-apisix
       args:
-        - APISIX_VERSION=2.1
+        - APISIX_VERSION=master
     restart: always
     volumes:
-      - ./apisix_config.yaml:/usr/local/apisix/conf/config.yaml:ro
+      - ./apisix_config2.yaml:/usr/local/apisix/conf/config.yaml:ro

Review comment:
       Need set apisix instance id.




----------------------------------------------------------------
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] starsz commented on a change in pull request #958: feat: implement API to get apisix instances status

Posted by GitBox <gi...@apache.org>.
starsz commented on a change in pull request #958:
URL: https://github.com/apache/apisix-dashboard/pull/958#discussion_r545604401



##########
File path: api/test/docker/apisix_config2.yaml
##########
@@ -93,4 +93,5 @@ plugin_attr:
 
   server-info:
     report_interval: 10
-    report_ttl: 3600
\ No newline at end of file
+    report_ttl: 3600
+

Review comment:
       I think it's not the business of this PR. Maybe we need another PR 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org