-
Notifications
You must be signed in to change notification settings - Fork 430
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
Fixed checkout_timeout on errors caused by not canceling the checkout #656
Conversation
@IceDragon200 Thank for the change! However, there is an issue in the CI with your change:
|
At a glance it seems the connection is being held over from the previous test (queue_timeout), EDIT: managed to get the tests running on my local, so it seems when one or the other runs before the connection isn't being returned EDIT.2: It's broken on master as well |
@IceDragon200 testing isolation issues? |
@isaacsanders It seems like it, and the problem exists in master as well, I haven't had time to revisit the issue due to work, benotic said he would look into it later, but seems he's busy as well |
Here's an idea after reading the tests: the |
This fix has a drawback of not removing request state stored in a process dict (could become a problem for a long-living process) and
I tried to fix it in #661 |
#661 Solved the issue, so my PR is no longer needed |
Related #643
Hopefully this PR should resolve the the issue with connections being exhausted socket/transport errors, I also did a very, very minuscule bit of code gardening