-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
[3007.x] Fix #66194: Exchange HTTPClient by AsyncHTTPClient in salt.utils.http #66330
base: 3007.x
Are you sure you want to change the base?
Conversation
Hey @krombel , since this effects 3007, would you mind rebasing this against the 3007.x branch? |
@twangboy done |
@krombel Would you know how to write a test for this? |
I attempted this patch but got this error:
|
@dwoz Anything I can do to help bring this PR forward? As mentioned my knowledge of testing that specific part is limited (as it is caused by concurrency which I do not know how to "create" it for a test case...) A previous version of this PR did not using the SyncWrapper caused this problem which got fixed in the meantime. (I am just curious how long I need to keep my workaround and don't want this to get overseen as the last "action" on this is almost a month ago...) |
Thanks @ITJamie 🎉 |
What does this PR do?
It replaces HTTPClient by AsyncHTTPClient
What issues does this PR fix or reference?
Fixes #66194
Background
tornadoweb/tornado#2325 (comment)
==> might be caused by #64304
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
Yes