apache / apisix-go-plugin-runner

Go Plugin Runner for APISIX
https://apisix.apache.org/
Apache License 2.0
167 stars 69 forks source link

feature: try to introduce context to plugin runner #63

Closed Belyenochi closed 2 years ago

Belyenochi commented 2 years ago

Resolves issues 34

codecov-commenter commented 2 years ago

Codecov Report

Merging #63 (fed69d0) into master (6bb1c4b) will increase coverage by 3.09%. The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #63      +/-   ##
==========================================
+ Coverage   77.74%   80.84%   +3.09%     
==========================================
  Files          11       11              
  Lines         701      710       +9     
==========================================
+ Hits          545      574      +29     
+ Misses        119      100      -19     
+ Partials       37       36       -1     
Impacted Files Coverage Δ
internal/http/request.go 94.28% <88.88%> (-0.25%) :arrow_down:
internal/http/response.go 100.00% <0.00%> (ø)
internal/server/server.go 67.47% <0.00%> (+17.07%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6bb1c4b...fed69d0. Read the comment docs.

Belyenochi commented 2 years ago

Don't forget to add a test to cover it.

Yes, I added a simple Context unit test, please feel free to drop me a message if you have good optimization suggestions, thanks

Belyenochi commented 2 years ago

Need to set the ctx to nil in

https://github.com/apache/apisix-go-plugin-runner/blob/1a00a4b14f930c4588ede207c18c341336a760fd/internal/http/request.go#L160

Thanks for the code review, done fixed

Belyenochi commented 2 years ago

approve running workflows.

pls approve running workflows.

Belyenochi commented 2 years ago

Need to set the ctx to nil in

https://github.com/apache/apisix-go-plugin-runner/blob/1a00a4b14f930c4588ede207c18c341336a760fd/internal/http/request.go#L160

pls help to approve again

Belyenochi commented 2 years ago

hi @spacewander, pls help to approve running workflows

spacewander commented 2 years ago

Merged. Thanks!