-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[grid] keep HttpClient alive until unused #12558 #12978
Conversation
An alternative way to handle this might be to use a WeakReference in the cache and use a Cleaner to call |
a212e00
to
cf97bb1
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## trunk #12978 +/- ##
=======================================
Coverage 57.52% 57.52%
=======================================
Files 86 86
Lines 5299 5299
Branches 221 221
=======================================
Hits 3048 3048
Misses 2030 2030
Partials 221 221 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please solve the conflict so we merge this?
70e8801
to
929debd
Compare
@diemol i have fixed it, the PR should have no conflicts left. |
There are some failures in CI, do you know if they are related? |
latest on trunk broke the CI so that might be causing these failures. |
i am currently looking into it |
This reverts commit ce7cfc8.
09beebe
to
e291460
Compare
Description
The old implementation did close the HttpClient some minutes after access and did not check if the HttpClient has request still waiting for a response.
Motivation and Context
This did break long running requests, especially when the client has a high read timeout set.
Note: This PR might make #12975 even worse, as it will not close the http client and prevent the retry to send the command more often.
Note: This PR will also fix the broken get session url endpoint and uses it where ever it makes sense.
This might lead to issues when people are using a grid of mixed versions, but is this supported at all?
Note: This PR will also add more output when the JSON parser failed, as it was useful to debug the broken get session url endpoint and might be useful for other issues too.has been removed from the PR as requested.Types of changes
Checklist