Default to HTTP/2 on dotnet5 #590
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brendandburns The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
k8s-ci-robot
added
size/S
k8s-ci-robot
added
size/M
brendandburns
requested review from
tg123
and removed request for
krabhishek8260
|
let me cherrypick and test |
|
yeah, there's no version negotiation in HTTP :) |
| var sh = new SocketsHttpHandler | ||
| { | ||
| KeepAlivePingPolicy = HttpKeepAlivePingPolicy.WithActiveRequests, | ||
| KeepAlivePingDelay = new TimeSpan(0, 3, 0), // Send pings every three minutes |
There was a problem hiding this comment.
| KeepAlivePingDelay = new TimeSpan(0, 3, 0), // Send pings every three minutes | |
| KeepAlivePingDelay = TimeSpan.FromMinutes(3), // Send pings every three minutes |
| { | ||
| KeepAlivePingPolicy = HttpKeepAlivePingPolicy.WithActiveRequests, | ||
| KeepAlivePingDelay = new TimeSpan(0, 3, 0), // Send pings every three minutes | ||
| KeepAlivePingTimeout = new TimeSpan(0, 0, 30), // Timeout pings after 30s of no response |
There was a problem hiding this comment.
| KeepAlivePingTimeout = new TimeSpan(0, 0, 30), // Timeout pings after 30s of no response | |
| KeepAlivePingTimeout = TimeSpan.FromSeconds(30), // Timeout pings after 30s of no response |
| @@ -243,6 +246,10 @@ private void CreateHttpClient(DelegatingHandler[] handlers, KubernetesClientConf | |||
|
|
|||
| AppendDelegatingHandler<WatcherDelegatingHandler>(); | |||
| HttpClient = new HttpClient(FirstMessageHandler, false); | |||
|
|
|||
| #if NET5_0 | |||
| HttpClient.DefaultRequestVersion = HttpVersion.Version20; | |||
There was a problem hiding this comment.
This does NOT work,
see https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient.defaultrequestversion?view=net-5.0
The DefaultRequestVersion property doesn't apply to the SendAsync method. The HttpRequestMessage parameter passed as the argument to the SendAsync method has its own Version property that controls the HTTP version used for the request.
Unfortunately, our generated code use SendAsync LOL
Here is my test (Should find a way to do automation)
- create a AKS (1.19)
- disable tcp keepalive
- start watch example
got a [R] from remote side after 5 min and then watch will not show any event from kubectl run
02:42:58.973029 IP 10.x.x.x.49746 > 52.x.x.x.443: Flags [.], ack 28897, win 492, options [nop,nop,TS val 255369207 ecr 3243887534], length 0
02:47:48.004159 IP 52.x.x.x.443 > 10.x.x.x.49746: Flags [R], seq 660228079, win 0, length 0
|
please pick #592 into this PR |
|
@brendandburns: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
1 similar comment
|
@brendandburns: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
the
needs-rebase
No description provided.