You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "TomCN0803 (via GitHub)" <gi...@apache.org> on 2023/11/18 07:53:27 UTC
[PR] feat: support request body rewrite [apisix-go-plugin-runner]
TomCN0803 opened a new pull request, #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151
Features and modifications includes:
- Update github.com/api7/ext-plugin-proto to v0.6.1
- Add SetBody([]byte) to http.Request
- Add a sample request body rewrite plugin
- Add an e2e test for the sample request body rewrite plugin
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support request body rewrite #127 [apisix-go-plugin-runner]
Posted by "TomCN0803 (via GitHub)" <gi...@apache.org>.
TomCN0803 commented on PR #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151#issuecomment-1823873189
> Whether content-length needs to be processed？
Is it a must to make limitations here? Flatbuffer itself has size limit of 2GB (https://stackoverflow.com/questions/59185516/do-flatbuffers-have-size-limits).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support request body rewrite #127 [apisix-go-plugin-runner]
Posted by "TomCN0803 (via GitHub)" <gi...@apache.org>.
TomCN0803 commented on code in PR #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151#discussion_r1401408114
##########
.gitignore:
##########
@@ -7,3 +7,6 @@ release/
coverage.txt
logs/
*.svg
+/.vscode
+go.work
+go.work.sum
Review Comment:
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support request body rewrite #127 [apisix-go-plugin-runner]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151#discussion_r1400328847
##########
cmd/go-runner/plugins/limit_req.go:
##########
@@ -5,7 +5,7 @@
// (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
+// http://www.apache.org/licenses/LICENSE-2.0
Review Comment:
Why do you modify this?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support request body rewrite #127 [apisix-go-plugin-runner]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151#discussion_r1400622303
##########
.gitignore:
##########
@@ -7,3 +7,6 @@ release/
coverage.txt
logs/
*.svg
+/.vscode
+go.work
+go.work.sum
Review Comment:
Why do we need to ignore these three files? how are they created? Why do we ignore the file under the root directory `/.vscode`?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support request body rewrite #127 [apisix-go-plugin-runner]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151#issuecomment-1822007415
Nice work!
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support request body rewrite #127 [apisix-go-plugin-runner]
Posted by "TomCN0803 (via GitHub)" <gi...@apache.org>.
TomCN0803 commented on code in PR #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151#discussion_r1401408114
##########
.gitignore:
##########
@@ -7,3 +7,6 @@ release/
coverage.txt
logs/
*.svg
+/.vscode
+go.work
+go.work.sum
Review Comment:
🆗
##########
.gitignore:
##########
@@ -7,3 +7,6 @@ release/
coverage.txt
logs/
*.svg
+/.vscode
+go.work
+go.work.sum
Review Comment:
🆗
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support request body rewrite #127 [apisix-go-plugin-runner]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151#issuecomment-1818230593
## [Codecov](https://app.codecov.io/gh/apache/apisix-go-plugin-runner/pull/151?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: `5 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`f049304`)](https://app.codecov.io/gh/apache/apisix-go-plugin-runner/commit/f04930485b11ec29121585b2864157c957602be8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 80.75% compared to head [(`ef15c7d`)](https://app.codecov.io/gh/apache/apisix-go-plugin-runner/pull/151?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 80.37%.
> :exclamation: Current head ef15c7d differs from pull request most recent head 3b80916. Consider uploading reports for the commit 3b80916 to get more accurate results
| [Files](https://app.codecov.io/gh/apache/apisix-go-plugin-runner/pull/151?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [cmd/go-runner/plugins/request\_body\_rewrite.go](https://app.codecov.io/gh/apache/apisix-go-plugin-runner/pull/151?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y21kL2dvLXJ1bm5lci9wbHVnaW5zL3JlcXVlc3RfYm9keV9yZXdyaXRlLmdv) | 75.00% | [2 Missing and 2 partials :warning: ](https://app.codecov.io/gh/apache/apisix-go-plugin-runner/pull/151?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [internal/http/request.go](https://app.codecov.io/gh/apache/apisix-go-plugin-runner/pull/151?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-aW50ZXJuYWwvaHR0cC9yZXF1ZXN0Lmdv) | 90.90% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/apisix-go-plugin-runner/pull/151?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #151 +/- ##
==========================================
- Coverage 80.75% 80.37% -0.38%
==========================================
Files 16 17 +1
Lines 1034 1060 +26
==========================================
+ Hits 835 852 +17
- Misses 146 156 +10
+ Partials 53 52 -1
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/apisix-go-plugin-runner/pull/151?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support request body rewrite #127 [apisix-go-plugin-runner]
Posted by "TomCN0803 (via GitHub)" <gi...@apache.org>.
TomCN0803 commented on code in PR #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151#discussion_r1400644345
##########
.gitignore:
##########
@@ -7,3 +7,6 @@ release/
coverage.txt
logs/
*.svg
+/.vscode
+go.work
+go.work.sum
Review Comment:
- go.work & go.work.sum: Are go workspace related files (https://go.dev/doc/tutorial/workspaces) which should be gitignored. Go workspace is used to work with multiple go modules and our project contains two go modules (project root and ./tests/e2e);
- .vscode: Is vscode editor related folder that contains user specific vscode settings, which definately shouldn't be committed.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support request body rewrite #127 [apisix-go-plugin-runner]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151#discussion_r1401405416
##########
.gitignore:
##########
@@ -7,3 +7,6 @@ release/
coverage.txt
logs/
*.svg
+/.vscode
+go.work
+go.work.sum
Review Comment:
OK, i think `/.vscode` should be `.vscode`, `/.vscode` is refer to the directory `.vscode` under the root directory
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support request body rewrite #127 [apisix-go-plugin-runner]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151#discussion_r1400622303
##########
.gitignore:
##########
@@ -7,3 +7,6 @@ release/
coverage.txt
logs/
*.svg
+/.vscode
+go.work
+go.work.sum
Review Comment:
Why do we need to ignore these three files? how are they created?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support request body rewrite #127 [apisix-go-plugin-runner]
Posted by "TomCN0803 (via GitHub)" <gi...@apache.org>.
TomCN0803 commented on code in PR #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151#discussion_r1400408460
##########
cmd/go-runner/plugins/limit_req.go:
##########
@@ -5,7 +5,7 @@
// (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
+// http://www.apache.org/licenses/LICENSE-2.0
Review Comment:
Cuz when I ran `make lint` the golangci-lint will report this line not "go-imported". And when I "format code" with my VSCode editor it automatically formats the code.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support request body rewrite #127 [apisix-go-plugin-runner]
Posted by "TomCN0803 (via GitHub)" <gi...@apache.org>.
TomCN0803 commented on code in PR #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151#discussion_r1400408460
##########
cmd/go-runner/plugins/limit_req.go:
##########
@@ -5,7 +5,7 @@
// (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
+// http://www.apache.org/licenses/LICENSE-2.0
Review Comment:
Cuz when I ran `make lint` the golangci-lint will report this line not "goimports-ed". And when I "format code" with my VSCode editor it automatically formats the code.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support request body rewrite #127 [apisix-go-plugin-runner]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 merged PR #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support request body rewrite #127 [apisix-go-plugin-runner]
Posted by "monkeyDluffy6017 (via GitHub)" <gi...@apache.org>.
monkeyDluffy6017 commented on PR #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151#issuecomment-1822007653
@bzp2010 please help to review
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: support request body rewrite #127 [apisix-go-plugin-runner]
Posted by "soulbird (via GitHub)" <gi...@apache.org>.
soulbird commented on PR #151:
URL: https://github.com/apache/apisix-go-plugin-runner/pull/151#issuecomment-1823862681
Whether content-length needs to be processed？
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org