Fixing issue #7072 #7608
Fixing issue #7072 #7608
Conversation
psf-chronographer
bot
added
the
bot:chronographer:provided
for more information, see https://pre-commit.ci
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #7608 +/- ##
==========================================
- Coverage 97.35% 97.35% -0.01%
==========================================
Files 106 106
Lines 31513 31514 +1
Branches 3589 3589
==========================================
Hits 30679 30679
- Misses 630 631 +1
Partials 204 204
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Thanks, do you think you could turn your reproducer into a test? |
|
No I'm sorry I don't know how to do that. |
|
It looks like your reproducer just mocks out a method before making a request. You can mock it out properly in a test with mock.patch(). You can add a |
| @@ -0,0 +1 @@ | |||
| Fixes the "Task was destroyed but it is pending" issue related to `TCPConnector._resolve_host` taking too long (probably because of a DNS lookup failure) | |||
There was a problem hiding this comment.
Additionally to what Sam asked regarding adding a regression test, this change note must use RST syntax (hence double backticks, not single). And please, check out https://docs.aiohttp.org/en/stable/contributing.html#adding-change-notes-with-your-prs for the writing style to be used in the changelog. For example, in order for it to be coherent, the past tense should be used (or variants of the present tense referring to the changes as compared to the previous release).
You can also optionally add a byline. It's all described in that document.
| @@ -1107,6 +1107,7 @@ def drop_exception(fut: "asyncio.Future[List[Dict[str, Any]]]") -> None: | |||
| fut.result() | |||
|
|
|||
| host_resolved.add_done_callback(drop_exception) | |||
| host_resolved.cancel() | |||
There was a problem hiding this comment.
Looking at this again, the comment above very clearly says we shouldn't cancel this task, which is why it's shielded. Without a test, I don't think this will be going anywhere.
Dreamsorcerer
added
the
pr-unfinished
|
Superseded by #8967. |
Under some specific circumstances, _resolve_host can fail, which will lead to an exception
Task was destroyed but it is pending!.This PR fixes the issue.
What do these changes do?
Following suggestion from @michaelzus, applying
host_resolved.cancel()when a CancelledError occurs stop the above issue to be raised.Are there changes in behavior for the user?
None
Related issue number
Fixes #7072
Checklist
CONTRIBUTORS.txtCHANGESfolder<issue_id>.<type>for example (588.bugfix)issue_idchange it to the pr id after creating the pr.feature: Signifying a new feature..bugfix: Signifying a bug fix..doc: Signifying a documentation improvement..removal: Signifying a deprecation or removal of public API..misc: A ticket has been closed, but it is not of interest to users.