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\-\=\.\"\?\,\'\/\\\+&%$#_]*)?$/;` 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\-\=\.\"\?\,\'\/\\\+&%$#_]*)?$/;` 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