Don't cancel tasks when entering a timer context #5853

Dreamsorcerer · 2021-07-03T21:56:08Z

codecov · 2021-07-03T22:12:40Z

Codecov Report

Merging #5853 (e13da75) into master (3eb0121) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #5853      +/-   ##
==========================================
- Coverage   93.31%   93.31%   -0.01%     
==========================================
  Files         102      102              
  Lines       30066    30065       -1     
  Branches     2689     2689              
==========================================
- Hits        28057    28056       -1     
  Misses       1833     1833              
  Partials      176      176

Flag	Coverage Δ
unit	`93.24% <100.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
aiohttp/helpers.py	`96.87% <ø> (-0.01%)`	⬇️
tests/test_helpers.py	`98.84% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3eb0121...e13da75. Read the comment docs.

CHANGES/5853.bugfix

webknjaz · 2021-07-05T15:26:55Z

I'm not sure how to assess this so I'll wait for @asvetlov to take a look.

webknjaz · 2021-10-20T12:08:33Z

@Dreamsorcerer this should probably be rebased with all the CI updates that's happened on master.

asvetlov

Thanks, @Dreamsorcerer
I saw cancellation events in logs on my job but never had enough time to dig into the problem.
To the note: the cancellation behavior change was introduced in 3.7, earlier aiohttp versions have different logic.

CHANGES/5853.bugfix

patchback · 2021-10-23T16:33:54Z

Backport to 3.8: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply c091db8 on top of patchback/backports/3.8/c091db813c01e48140bc8146d3ca1d80bb1e9578/pr-5853

Backporting merged PR #5853 into master

Ensure you have a local repo clone of your fork. Unless you cloned it
from the upstream, this would be your origin remote.
Make sure you have an upstream repo added as a remote too. In these
instructions you'll refer to it by the name upstream. If you don't
have it, here's how you can add it:
```
$ git remote add upstream https://github.com/aio-libs/aiohttp.git
```

Ensure you have the latest copy of upstream and prepare a branch
that will hold the backported code:

$ git fetch upstream
$ git checkout -b patchback/backports/3.8/c091db813c01e48140bc8146d3ca1d80bb1e9578/pr-5853 upstream/3.8

Now, cherry-pick PR Don't cancel tasks when entering a timer context #5853 contents into that branch:
```
$ git cherry-pick -x c091db813c01e48140bc8146d3ca1d80bb1e9578
```
If it'll yell at you with something like fatal: Commit c091db813c01e48140bc8146d3ca1d80bb1e9578 is a merge but no -m option was given., add -m 1 as follows intead:
```
$ git cherry-pick -m1 -x c091db813c01e48140bc8146d3ca1d80bb1e9578
```
At this point, you'll probably encounter some merge conflicts. You must
resolve them in to preserve the patch from PR Don't cancel tasks when entering a timer context #5853 as close to the
original as possible.

Push this branch to your fork on GitHub:

$ git push origin patchback/backports/3.8/c091db813c01e48140bc8146d3ca1d80bb1e9578/pr-5853

Create a PR, ensure that the CI is green. If it's not — update it so that
the tests and any other checks pass. This is it!
Now relax and wait for the maintainers to process your pull request
when they have some cycles to do reviews. Don't worry — they'll tell you if
any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

aio-libs-github-bot · 2021-10-23T16:34:12Z

💔 Backport was not successful

The PR was attempted backported to the following branches:

❌ 3.8: Commit could not be cherrypicked due to conflicts

asvetlov · 2021-10-23T16:44:41Z

Backport is here: a4b7505

(Note this depends on and extends #5853) When reading in a loop while the buffer is being constantly filled, the timeout does not work as there are no calls to `_wait()` where the timer is used. I don't know if this edge case is enough to be worried about, but have put together an initial attempt at fixing it. I'm not sure if this is really the right solution, but can atleast be used as as a discussion on ways to improve this. This can't be backported as this changes the public API (one of the functions is now async). Related #5851. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

(Note this depends on and extends #5853) When reading in a loop while the buffer is being constantly filled, the timeout does not work as there are no calls to `_wait()` where the timer is used. I don't know if this edge case is enough to be worried about, but have put together an initial attempt at fixing it. I'm not sure if this is really the right solution, but can atleast be used as as a discussion on ways to improve this. This can't be backported as this changes the public API (one of the functions is now async). Related #5851. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> (cherry picked from commit 80e2bde)

(Note this depends on and extends #5853) When reading in a loop while the buffer is being constantly filled, the timeout does not work as there are no calls to `_wait()` where the timer is used. I don't know if this edge case is enough to be worried about, but have put together an initial attempt at fixing it. I'm not sure if this is really the right solution, but can atleast be used as as a discussion on ways to improve this. This can't be backported as this changes the public API (one of the functions is now async). Related #5851. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> (cherry picked from commit 80e2bde)  ## What do these changes do?  ## Are there changes in behavior for the user?  ## Related issue number  ## Checklist - [ ] I think the code is well written - [ ] Unit tests for the changes exist - [ ] Documentation reflects the changes - [ ] If you provide code modification, please add yourself to `CONTRIBUTORS.txt` * The format is <Name> <Surname>. * Please keep alphabetical order, the file is sorted by names. - [ ] Add a new news fragment into the `CHANGES` folder * name it `<issue_id>.<type>` for example (588.bugfix) * if you don't have an `issue_id` change it to the pr id after creating the pr * ensure type is one of the following: * `.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. * Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

Dreamsorcerer added 2 commits July 3, 2021 22:55

Don't cancel tasks when entering a timer context

cfbeab0

Update test_helpers.py

31f3b54

Create 5853.bugfix

6835dab

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Jul 4, 2021

Dreamsorcerer marked this pull request as ready for review July 4, 2021 12:07

Dreamsorcerer requested a review from asvetlov as a code owner July 4, 2021 12:07

Dreamsorcerer mentioned this pull request Jul 4, 2021

Allow timeout to work when reading with nowait #5854

Merged

webknjaz reviewed Jul 5, 2021

View reviewed changes

CHANGES/5853.bugfix Show resolved Hide resolved

Dreamsorcerer mentioned this pull request Jul 20, 2021

Streamed response truncated on Alpine 3.13/3.14 #5902

Closed

1 task

Dreamsorcerer added the backport-3.8 label Sep 30, 2021

Dreamsorcerer mentioned this pull request Sep 30, 2021

Add support for Python 3.10 and musl-based runtimes aio-libs/multidict#629

Merged

asvetlov added this to the 3.8 milestone Oct 1, 2021

Merge branch 'master' into Dreamsorcerer-patch-4

e13da75

Dreamsorcerer removed the bot:chronographer:provided There is a change note present in this PR label Oct 20, 2021

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Oct 20, 2021

asvetlov approved these changes Oct 23, 2021

View reviewed changes

CHANGES/5853.bugfix Show resolved Hide resolved

asvetlov merged commit c091db8 into master Oct 23, 2021

asvetlov deleted the Dreamsorcerer-patch-4 branch October 23, 2021 16:33

asvetlov pushed a commit that referenced this pull request Oct 23, 2021

Don't cancel tasks when entering a timer context (#5853)

a4b7505

Dreamsorcerer mentioned this pull request May 16, 2023

Allow timeout to work when reading with nowait (#5854) #7292

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Don't cancel tasks when entering a timer context #5853

Don't cancel tasks when entering a timer context #5853

Dreamsorcerer commented Jul 3, 2021 •

edited

Loading

codecov bot commented Jul 3, 2021 •

edited

Loading

webknjaz commented Jul 5, 2021

webknjaz commented Oct 20, 2021

asvetlov left a comment

patchback bot commented Oct 23, 2021 •

edited

Loading

aio-libs-github-bot bot commented Oct 23, 2021

asvetlov commented Oct 23, 2021

Don't cancel tasks when entering a timer context #5853

Don't cancel tasks when entering a timer context #5853

Conversation

Dreamsorcerer commented Jul 3, 2021 • edited Loading

codecov bot commented Jul 3, 2021 • edited Loading

Codecov Report

webknjaz commented Jul 5, 2021

webknjaz commented Oct 20, 2021

asvetlov left a comment

Choose a reason for hiding this comment

patchback bot commented Oct 23, 2021 • edited Loading

Backport to 3.8: 💔 cherry-picking failed — conflicts found

Backporting merged PR #5853 into master

aio-libs-github-bot bot commented Oct 23, 2021

💔 Backport was not successful

asvetlov commented Oct 23, 2021

Dreamsorcerer commented Jul 3, 2021 •

edited

Loading

codecov bot commented Jul 3, 2021 •

edited

Loading

patchback bot commented Oct 23, 2021 •

edited

Loading