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/04/09 03:28:00 UTC
[GitHub] [apisix-ingress-controller] tokers opened a new pull request #347: fix: pass remote_addrs to APISIX
tokers opened a new pull request #347:
URL: https://github.com/apache/apisix-ingress-controller/pull/347
Please answer these questions before submitting a pull request
- Why submit this pull request?
- [x] Bugfix
- [ ] New feature provided
- [ ] Improve performance
- [ ] Backport patches
- Related issues
___
### Bugfix
- Description
- How to fix?
___
### New feature or improvement
- Describe the details and related test reports.
___
### Backport patches
- Why need to backport?
- Source branch
- Related commits and pull requests
- Target 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-ingress-controller] codecov-io commented on pull request #347: fix: pass remote_addrs to APISIX
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #347:
URL: https://github.com/apache/apisix-ingress-controller/pull/347#issuecomment-816375774
# [Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/347?src=pr&el=h1) Report
> Merging [#347](https://codecov.io/gh/apache/apisix-ingress-controller/pull/347?src=pr&el=desc) (94c46ed) into [master](https://codecov.io/gh/apache/apisix-ingress-controller/commit/456efa6836e3195cc0fcb6603fb9243c1033860a?el=desc) (456efa6) will **decrease** coverage by `0.43%`.
> The diff coverage is `84.44%`.
[](https://codecov.io/gh/apache/apisix-ingress-controller/pull/347?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #347 +/- ##
==========================================
- Coverage 43.84% 43.40% -0.44%
==========================================
Files 40 39 -1
Lines 3435 3449 +14
==========================================
- Hits 1506 1497 -9
- Misses 1761 1781 +20
- Partials 168 171 +3
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-ingress-controller/pull/347?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/kube/translation/apisix\_route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/347/diff?src=pr&el=tree#diff-cGtnL2t1YmUvdHJhbnNsYXRpb24vYXBpc2l4X3JvdXRlLmdv) | `46.08% <12.50%> (-1.29%)` | :arrow_down: |
| [pkg/apisix/resource.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/347/diff?src=pr&el=tree#diff-cGtnL2FwaXNpeC9yZXNvdXJjZS5nbw==) | `78.64% <100.00%> (+0.20%)` | :arrow_up: |
| [pkg/apisix/route.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/347/diff?src=pr&el=tree#diff-cGtnL2FwaXNpeC9yb3V0ZS5nbw==) | `44.17% <100.00%> (-1.17%)` | :arrow_down: |
| [pkg/kube/translation/util.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/347/diff?src=pr&el=tree#diff-cGtnL2t1YmUvdHJhbnNsYXRpb24vdXRpbC5nbw==) | `100.00% <100.00%> (ø)` | |
| [pkg/apisix/cluster.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/347/diff?src=pr&el=tree#diff-cGtnL2FwaXNpeC9jbHVzdGVyLmdv) | `28.44% <0.00%> (-5.97%)` | :arrow_down: |
| [test/e2e/e2e.go](https://codecov.io/gh/apache/apisix-ingress-controller/pull/347/diff?src=pr&el=tree#diff-dGVzdC9lMmUvZTJlLmdv) | | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-ingress-controller/pull/347?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-ingress-controller/pull/347?src=pr&el=footer). Last update [456efa6...94c46ed](https://codecov.io/gh/apache/apisix-ingress-controller/pull/347?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-ingress-controller] jbampton commented on a change in pull request #347: fix: pass remote_addrs to APISIX
Posted by GitBox <gi...@apache.org>.
jbampton commented on a change in pull request #347:
URL: https://github.com/apache/apisix-ingress-controller/pull/347#discussion_r610394663
##########
File path: pkg/apisix/resource.go
##########
@@ -73,17 +73,18 @@ type item struct {
}
type routeItem struct {
- UpstreamId string `json:"upstream_id"`
- ServiceId string `json:"service_id"`
- Host string `json:"host"`
- Hosts []string `json:"hosts"`
- URI string `json:"uri"`
- Vars [][]v1.StringOrSlice `json:"vars"`
- Uris []string `json:"uris"`
- Desc string `json:"desc"`
- Methods []string `json:"methods"`
- Priority int `json:"priority"`
- Plugins map[string]interface{} `json:"plugins"`
+ UpstreamId string `json:"upstream_id"`
Review comment:
Generally I like some kind of order in lists, arrays or structures but not sure how this affects the whole system.
So with ordering some people chose an alphabetically ordered list, and others use a custom sort order for maybe the first few items in the list and then alphabetically for the rest of the items in the list. There are many ways to sort and design data structures.
Ordering also makes it easier to find existing items in the lists and speeds up adding new items.
So in this file there seems to be two `lists` and some other files in this PR have `lists`.
A general code style comment that I always push.
--
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-ingress-controller] tokers merged pull request #347: fix: pass remote_addrs to APISIX
Posted by GitBox <gi...@apache.org>.
tokers merged pull request #347:
URL: https://github.com/apache/apisix-ingress-controller/pull/347
--
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-ingress-controller] tokers commented on pull request #347: fix: pass remote_addrs to APISIX
Posted by GitBox <gi...@apache.org>.
tokers commented on pull request #347:
URL: https://github.com/apache/apisix-ingress-controller/pull/347#issuecomment-816409757
@gxthrj Please take a look when you have time.
--
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-ingress-controller] tokers commented on a change in pull request #347: fix: pass remote_addrs to APISIX
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #347:
URL: https://github.com/apache/apisix-ingress-controller/pull/347#discussion_r610466469
##########
File path: pkg/apisix/resource.go
##########
@@ -73,17 +73,18 @@ type item struct {
}
type routeItem struct {
- UpstreamId string `json:"upstream_id"`
- ServiceId string `json:"service_id"`
- Host string `json:"host"`
- Hosts []string `json:"hosts"`
- URI string `json:"uri"`
- Vars [][]v1.StringOrSlice `json:"vars"`
- Uris []string `json:"uris"`
- Desc string `json:"desc"`
- Methods []string `json:"methods"`
- Priority int `json:"priority"`
- Plugins map[string]interface{} `json:"plugins"`
+ UpstreamId string `json:"upstream_id"`
Review comment:
The order of fields affects the memory alignment but is not so important here, if that's really vital, we can optimize it in the future.
--
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