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 2021/02/03 06:16:50 UTC

[GitHub] [apisix-dashboard] Jaycean opened a new pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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


   Please answer these questions before submitting a pull request
   
   - Why submit this pull request?
   - [ ] Bugfix
   - [ ] New feature provided
   - [ ] Improve performance
   - [ ] Backport patches
   
   - Related issues
   #698 
   ___
   ### Chore
   - use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`
   - manage-api-conf.yaml Keep the local test for use, and then put the E2E test step into the makefile for execution, and then delete 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] Jaycean commented on a change in pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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



##########
File path: .github/workflows/deploy-with-docker.yml
##########
@@ -30,6 +30,8 @@ jobs:
         run: |
           sed -i 's/127.0.0.1:2379/172.16.238.10:2379/' api/conf/conf.yaml
           sed -i 's/host: 127.0.0.1/host: 0.0.0.0/' api/conf/conf.yaml
+          sed -i '/172.16.238.11:2379/d' api/conf/conf.yaml
+          sed -i '/172.16.238.12:2379/d' api/conf/conf.yaml

Review comment:
       @nic-chen This modification was added before(d07c1307ae75153d360749468091ef1e8e7bc514). @imjoey  didn't think it was necessary to add it, but it has been removed after discussing with you.




----------------------------------------------------------------
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] Jaycean commented on pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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


   ping @nic-chen @imjoey 
   I'm not sure that every job uses conf.yaml And whether mutual modification has an impact. So I'm interested in deploy-with- docker.yaml More than one configuration has been deleted.


----------------------------------------------------------------
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] Jaycean commented on a change in pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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



##########
File path: docs/back-end-e2e.md
##########
@@ -78,6 +78,18 @@ This document describes how to use E2E test locally.
 
 1. [install docker-compose](https://docs.docker.com/compose/install/)
 
+**NOTE:** To run docker compose locally, you need to modify ./api/conf/conf.yaml 'host' and 'endpoints' configuration in the file

Review comment:
       Thks,Your translation is better than mine. Done.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [apisix-dashboard] nic-chen commented on a change in pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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



##########
File path: .github/workflows/deploy-with-docker.yml
##########
@@ -30,6 +30,8 @@ jobs:
         run: |
           sed -i 's/127.0.0.1:2379/172.16.238.10:2379/' api/conf/conf.yaml
           sed -i 's/host: 127.0.0.1/host: 0.0.0.0/' api/conf/conf.yaml
+          sed -i '/172.16.238.11:2379/d' api/conf/conf.yaml
+          sed -i '/172.16.238.12:2379/d' api/conf/conf.yaml

Review comment:
       @imjoey  sorry, I don't get the point.. why we should change `deploy-with-docker.yml`, it's not related to this pr.
   




----------------------------------------------------------------
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] imjoey commented on a change in pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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



##########
File path: docs/back-end-e2e.md
##########
@@ -78,6 +78,18 @@ This document describes how to use E2E test locally.
 
 1. [install docker-compose](https://docs.docker.com/compose/install/)
 
+**NOTE:** To run docker compose locally, you need to modify ./api/conf/conf.yaml 'host' and 'endpoints' configuration in the file

Review comment:
       > Thks, It‘s better than mine. Done.
   
   @Jaycean thanks.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [apisix-dashboard] Jaycean commented on pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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


   > We can remove the file `manager-api-conf.yaml` right?
   
   After discussing this problem with @nic-chen , We plan not to delete it for the time being. We will submit a new PR, add the relevant steps of E2E test to the makefile, and then delete the 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] imjoey commented on a change in pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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



##########
File path: docs/back-end-e2e.md
##########
@@ -78,6 +78,18 @@ This document describes how to use E2E test locally.
 
 1. [install docker-compose](https://docs.docker.com/compose/install/)
 
+**NOTE:** To run docker compose locally, you need to modify ./api/conf/conf.yaml 'host' and 'endpoints' configuration in the file

Review comment:
       @Jaycean  How about here we say:  ( Thanks ) 
   
   
   **NOTE:** In order to run docker compose locally, please change the values of `listen.host` and `etcd.endpoints` within `./api/conf/conf.yaml` as follows:
   




----------------------------------------------------------------
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 #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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


   > ping @nic-chen @imjoey
   > I'm not sure that every job uses conf.yaml And whether mutual modification has an impact. So I'm interested in deploy-with- docker.yaml More than one configuration has been deleted.
   
   No, each CI is independent of each other
   


----------------------------------------------------------------
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 #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?src=pr&el=h1) Report
   > Merging [#1417](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?src=pr&el=desc) (2f60002) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/0a0a42ae1c81f21a03c98a991b707dba6b724364?el=desc) (0a0a42a) will **decrease** coverage by `21.40%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master    #1417       +/-   ##
   ===========================================
   - Coverage   66.59%   45.19%   -21.41%     
   ===========================================
     Files          44       35        -9     
     Lines        2937     2516      -421     
   ===========================================
   - Hits         1956     1137      -819     
   - Misses        744     1214      +470     
   + Partials      237      165       -72     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/filter/request\_id.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9yZXF1ZXN0X2lkLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (-70.41%)` | :arrow_down: |
   | [api/internal/filter/cors.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9jb3JzLmdv) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
   | [api/internal/filter/schema.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9zY2hlbWEuZ28=) | `0.00% <0.00%> (-55.47%)` | :arrow_down: |
   | [api/internal/handler/upstream/upstream.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvdXBzdHJlYW0vdXBzdHJlYW0uZ28=) | `28.81% <0.00%> (-54.24%)` | :arrow_down: |
   | [api/internal/utils/consts/api\_error.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL2NvbnN0cy9hcGlfZXJyb3IuZ28=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [api/internal/handler/data\_loader/route\_import.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvZGF0YV9sb2FkZXIvcm91dGVfaW1wb3J0Lmdv) | `27.41% <0.00%> (-37.50%)` | :arrow_down: |
   | [api/internal/handler/server\_info/server\_info.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmVyX2luZm8vc2VydmVyX2luZm8uZ28=) | `57.14% <0.00%> (-33.34%)` | :arrow_down: |
   | [api/internal/utils/closer.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL2Nsb3Nlci5nbw==) | `0.00% <0.00%> (-33.34%)` | :arrow_down: |
   | ... and [30 more](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?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/1417?src=pr&el=footer). Last update [0a0a42a...2f60002](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?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] Jaycean commented on a change in pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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



##########
File path: docs/back-end-e2e.md
##########
@@ -78,6 +78,18 @@ This document describes how to use E2E test locally.
 
 1. [install docker-compose](https://docs.docker.com/compose/install/)
 
+**NOTE:** To run docker compose locally, you need to modify ./api/conf/conf.yaml 'host' and 'endpoints' configuration in the file

Review comment:
       Thks, It‘s better than mine. Done.
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [apisix-dashboard] starsz commented on pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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


   > > We can remove the file `manager-api-conf.yaml` right?
   > 
   > After discussing this problem with @nic-chen , We plan not to delete it for the time being. We will submit a new PR, add the relevant steps of E2E test to the makefile, and then delete the file.
   
   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 #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?src=pr&el=h1) Report
   > Merging [#1417](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?src=pr&el=desc) (71dd4c2) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/0a0a42ae1c81f21a03c98a991b707dba6b724364?el=desc) (0a0a42a) will **decrease** coverage by `21.40%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master    #1417       +/-   ##
   ===========================================
   - Coverage   66.59%   45.19%   -21.41%     
   ===========================================
     Files          44       35        -9     
     Lines        2937     2516      -421     
   ===========================================
   - Hits         1956     1137      -819     
   - Misses        744     1214      +470     
   + Partials      237      165       -72     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/filter/request\_id.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9yZXF1ZXN0X2lkLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (-70.41%)` | :arrow_down: |
   | [api/internal/filter/cors.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9jb3JzLmdv) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
   | [api/internal/filter/schema.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9zY2hlbWEuZ28=) | `0.00% <0.00%> (-55.47%)` | :arrow_down: |
   | [api/internal/handler/upstream/upstream.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvdXBzdHJlYW0vdXBzdHJlYW0uZ28=) | `28.81% <0.00%> (-54.24%)` | :arrow_down: |
   | [api/internal/utils/consts/api\_error.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL2NvbnN0cy9hcGlfZXJyb3IuZ28=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [api/internal/handler/data\_loader/route\_import.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvZGF0YV9sb2FkZXIvcm91dGVfaW1wb3J0Lmdv) | `27.41% <0.00%> (-37.50%)` | :arrow_down: |
   | [api/internal/handler/server\_info/server\_info.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmVyX2luZm8vc2VydmVyX2luZm8uZ28=) | `57.14% <0.00%> (-33.34%)` | :arrow_down: |
   | [api/internal/utils/closer.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL2Nsb3Nlci5nbw==) | `0.00% <0.00%> (-33.34%)` | :arrow_down: |
   | ... and [30 more](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?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/1417?src=pr&el=footer). Last update [0a0a42a...b8e40a1](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?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 #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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


   > The CI check `Code scanning results / CodeQL` suddenly failed due to the url regex `/^[0-9a-zA-Z]([-.\w]*[0-9a-zA-Z])*(:(0-9)*)*(\/?)([a-zA-Z0-9\-\=\.\"\?\,\'\/\\\+&amp;%$#_]*)?$/;` is an `Inefficient regular expression` at file `web/src/pages/Route/components/DebugViews/DebugDrawView.tsx`, please see https://github.com/apache/apisix-dashboard/pull/1417/checks?check_run_id=1823285766 for details.
   > 
   > While it's not changed within this PR and even it was last updated at about 9 days ago in [b0c531d](https://github.com/apache/apisix-dashboard/commit/b0c531d948dd62ed2e0393ffdf6b2e9fc7f4b15a) authored by @liuxiran . Not sure whether it's 100% reproducible and need more test results.
   > 
   > @nic-chen @liuxiran do you have any ideas about this? Thanks.
   
   
   
   We should determine whether this CI now fails every time, perhaps because the action library used has been changed? we could fix it in another PR.
   
   
   


----------------------------------------------------------------
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 #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?src=pr&el=h1) Report
   > Merging [#1417](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?src=pr&el=desc) (d07c130) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/0a0a42ae1c81f21a03c98a991b707dba6b724364?el=desc) (0a0a42a) will **decrease** coverage by `21.40%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff             @@
   ##           master    #1417       +/-   ##
   ===========================================
   - Coverage   66.59%   45.19%   -21.41%     
   ===========================================
     Files          44       35        -9     
     Lines        2937     2516      -421     
   ===========================================
   - Hits         1956     1137      -819     
   - Misses        744     1214      +470     
   + Partials      237      165       -72     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/filter/request\_id.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9yZXF1ZXN0X2lkLmdv) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [api/internal/core/store/storehub.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmVodWIuZ28=) | `0.00% <0.00%> (-70.41%)` | :arrow_down: |
   | [api/internal/filter/cors.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9jb3JzLmdv) | `0.00% <0.00%> (-66.67%)` | :arrow_down: |
   | [api/internal/filter/schema.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2ZpbHRlci9zY2hlbWEuZ28=) | `0.00% <0.00%> (-55.47%)` | :arrow_down: |
   | [api/internal/handler/upstream/upstream.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvdXBzdHJlYW0vdXBzdHJlYW0uZ28=) | `28.81% <0.00%> (-54.24%)` | :arrow_down: |
   | [api/internal/utils/consts/api\_error.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL2NvbnN0cy9hcGlfZXJyb3IuZ28=) | `0.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [api/internal/handler/data\_loader/route\_import.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvZGF0YV9sb2FkZXIvcm91dGVfaW1wb3J0Lmdv) | `27.41% <0.00%> (-37.50%)` | :arrow_down: |
   | [api/internal/handler/server\_info/server\_info.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmVyX2luZm8vc2VydmVyX2luZm8uZ28=) | `57.14% <0.00%> (-33.34%)` | :arrow_down: |
   | [api/internal/utils/closer.go](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL2Nsb3Nlci5nbw==) | `0.00% <0.00%> (-33.34%)` | :arrow_down: |
   | ... and [30 more](https://codecov.io/gh/apache/apisix-dashboard/pull/1417/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?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/1417?src=pr&el=footer). Last update [0a0a42a...d07c130](https://codecov.io/gh/apache/apisix-dashboard/pull/1417?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] imjoey merged pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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


   


----------------------------------------------------------------
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 #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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


   @imjoey Don't forget to delete useless commit info when merge commit.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [apisix-dashboard] nic-chen commented on pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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


   ping @imjoey 


----------------------------------------------------------------
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 #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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


   we could merge this PR first.
   
   @liuxiran  please help check CI `Code scanning results / CodeQL` when you have time. Thanks.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [apisix-dashboard] imjoey commented on a change in pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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



##########
File path: .github/workflows/deploy-with-docker.yml
##########
@@ -30,6 +30,8 @@ jobs:
         run: |
           sed -i 's/127.0.0.1:2379/172.16.238.10:2379/' api/conf/conf.yaml
           sed -i 's/host: 127.0.0.1/host: 0.0.0.0/' api/conf/conf.yaml
+          sed -i '/172.16.238.11:2379/d' api/conf/conf.yaml
+          sed -i '/172.16.238.12:2379/d' api/conf/conf.yaml

Review comment:
       @Jaycean IMHO, there is no need to remove the two lines here, as each job will be running in independent environment and configurations will not be affected by others.
   
   @nic-chen what's your opinions?




----------------------------------------------------------------
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] imjoey commented on pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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


   The CI check `Code scanning results / CodeQL` suddenly failed due to the url regex `/^[0-9a-zA-Z]([-.\w]*[0-9a-zA-Z])*(:(0-9)*)*(\/?)([a-zA-Z0-9\-\=\.\"\?\,\'\/\\\+&amp;%$#_]*)?$/;` is an `Inefficient regular expression` at  file `web/src/pages/Route/components/DebugViews/DebugDrawView.tsx`, please see https://github.com/apache/apisix-dashboard/pull/1417/checks?check_run_id=1823285766 for details.
   
   While it's not changed within this PR and even it was last updated at about 9 days ago in https://github.com/apache/apisix-dashboard/commit/b0c531d948dd62ed2e0393ffdf6b2e9fc7f4b15a authored by @liuxiran . Not sure whether it's 100% reproducible and need more test results. 
   
   @nic-chen @liuxiran  do you have any ideas about this? Thanks.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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



[GitHub] [apisix-dashboard] imjoey commented on pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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


   > @imjoey Don't forget to delete useless commit info when merge commit.
   
   @nic-chen sorry for that. I did in Github iOS app and will pay more attention on this. Thanks for reminding. 😄 


----------------------------------------------------------------
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] imjoey commented on a change in pull request #1417: chore: Use the original conf/config.yaml + sed to replace `api/test/docker/manager-api-conf.yaml`

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



##########
File path: .github/workflows/deploy-with-docker.yml
##########
@@ -30,6 +30,8 @@ jobs:
         run: |
           sed -i 's/127.0.0.1:2379/172.16.238.10:2379/' api/conf/conf.yaml
           sed -i 's/host: 127.0.0.1/host: 0.0.0.0/' api/conf/conf.yaml
+          sed -i '/172.16.238.11:2379/d' api/conf/conf.yaml
+          sed -i '/172.16.238.12:2379/d' api/conf/conf.yaml

Review comment:
       > @imjoey sorry, I don't get the point.. why we should change `deploy-with-docker.yml`, it's not related to this pr.
   
   @nic-chen 😄 just a misunderstanding. We both consider `deploy-with-docker.yml` should not be changed.




----------------------------------------------------------------
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