network: use http.ResponseController instead of GetHTTPRequestConnection #6044
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6 tasks
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6044 +/- ##
==========================================
- Coverage 56.15% 56.13% -0.03%
==========================================
Files 488 488
Lines 69532 69425 -107
==========================================
- Hits 39044 38969 -75
+ Misses 27832 27804 -28
+ Partials 2656 2652 -4 ☔ View full report in Codecov by Sentry. |
cce
force-pushed
the
http-responsecontroller
branch
from
b042bc0 to
d8c2922
Compare
gmalouf
approved these changes
Jul 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Mentioned in #6042, the network interface has since #1390 required a method
GetHTTPRequestConnectionso that an HTTP handler could call SetWriteDeadline to set a request-specific write deadline. (It is used for setting a longer write deadline when serving large catchpoint snapshot files.)In Go 1.20 per-request deadlines were added to the builtin HTTP library using http.ResponseController, which makes it very easy to do from inside a HTTP handler. This makes the connection-tracking middleware in #1390 and #5924 to implement
GossipNode.GetHTTPRequestConnectionobsolete.For reviewers: the deleted lines all come from #1390 and #5924, so if you open up those side-by-side you should be able to see what is being removed vs. what is being left.
Test Plan
Existing tests should pass, and maybe a new test should be added to ledgerService_test.go to ensure the write deadlines are working as described when using (http.ResponseController).SetWriteDeadline.