Opt-in to set Connection:Close on downstream requests #1441
Conversation
|
Build failed, probably fixed by #1436 ? |
Conflicts: src/Ocelot/Requester/HttpClientBuilder.cs Cherry picked from ThreeMammals#1441
|
Please do the following:
|
raman-m
force-pushed
the
feature/connection-close-opt-in
branch
from
d562ced to
05d0c97
Compare
raman-m
added
feature
|
@bjartekh Hi Bjarte! I see that develop branch in your fork is too old. Is this PR related to an issue in backlog? |
raman-m
requested review from
binarymash,
jps,
geffzhang,
RaynaldM,
philproctor,
anktsrkr,
sharpn,
abooraja,
wast,
raman-m,
yangzhongke and
thiagoloureiro
raman-m
force-pushed
the
feature/connection-close-opt-in
branch
from
05d0c97 to
b0b7f66
Compare
| handlers | ||
| .Select(handler => handler) | ||
| .Reverse() | ||
| .ToList() | ||
| .ForEach(handler => | ||
| { | ||
| var delegatingHandler = handler(); | ||
| delegatingHandler.InnerHandler = httpMessageHandler; | ||
| httpMessageHandler = delegatingHandler; | ||
| }); | ||
| return httpMessageHandler; | ||
| } |
There was a problem hiding this comment.
| handlers | |
| .Select(handler => handler) | |
| .Reverse() | |
| .ToList() | |
| .ForEach(handler => | |
| { | |
| var delegatingHandler = handler(); | |
| delegatingHandler.InnerHandler = httpMessageHandler; | |
| httpMessageHandler = delegatingHandler; | |
| }); | |
| return httpMessageHandler; | |
| } | |
| return handlers | |
| .Reverse() // Reverse the sequence | |
| .Aggregate(httpMessageHandler, (currentHandler, handlerFunc) => | |
| { | |
| var delegatingHandler = handlerFunc(); | |
| delegatingHandler.InnerHandler = currentHandler; | |
| return delegatingHandler; | |
| }); | |
| return httpMessageHandler; | |
| } |
There was a problem hiding this comment.
Yeap! ToList was a bit overhead to get access to ForEach helper.
Aggregate helper is good, for sure because it is applicable for this case.
There is another coding life hack: calling All to process all elements of collection. 😃
But!... Ray, this file is redundant one! I left this file during rebasing just to keep the author's changes: see // TODO 😉
Both files are just container of new changes
- src/Ocelot/Requester/HttpClientBuilder.cs
- src/Ocelot/Requester/HttpClientWrapper.cs
You may check, there are no these files in develop! 🙃
This PR is not ready for reviews: it must be developed more ❗
raman-m
force-pushed
the
feature/connection-close-opt-in
branch
from
b0b7f66 to
00469fc
Compare
raman-m
removed request for
philproctor,
anktsrkr,
sharpn,
abooraja,
wast,
raman-m,
yangzhongke and
thiagoloureiro
|
The branch has been rebased onto release/24.0 with top commit 59b63ea ! Next steps
|
raman-m
force-pushed
the
feature/connection-close-opt-in
branch
from
00469fc to
c3b2416
Compare
The build is 🟢 Wow! 🤯 😻 |
raman-m
force-pushed
the
feature/connection-close-opt-in
branch
from
c3b2416 to
2ec9ff1
Compare
raman-m
added
Configuration
…onClose = ConnectionClose ) in order to prevent persistent connections.
raman-m
force-pushed
the
feature/connection-close-opt-in
branch
from
2ec9ff1 to
b0879ea
Compare
|
@bjartekh, are you available? Could you please continue working on your pull request? |
Fixes / New Feature #
ConnectionClose: true|falseunderGlobalConfigurationor perRoutein the Ocelot configuration files.Proposed Changes
Side note: When benchmarking this on Azure Kubernetes Service (AKS), we observed that we got a higher throughput when running with Connnection:Close than using the
Ocelot.Provider.Kubernetespackage (which queries the Kubernetes API prior to sending each request). This is one scenario where this PR is beneficial.