Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove Request.wait_for_disconnection() method #8636

Merged
merged 6 commits into from
Aug 8, 2024
Merged

Remove Request.wait_for_disconnection() method #8636

merged 6 commits into from
Aug 8, 2024

Conversation

Dreamsorcerer
Copy link
Member

@Dreamsorcerer Dreamsorcerer commented Aug 7, 2024 •
edited
Loading

Fixes #8631.

@Dreamsorcerer Dreamsorcerer added backport-3.10 backport-3.11 Trigger automatic backporting to the 3.11 release branch by Patchback robot labels Aug 7, 2024
@Dreamsorcerer Dreamsorcerer requested a review from asvetlov as a code owner August 7, 2024 16:41
@Dreamsorcerer Dreamsorcerer requested a review from webknjaz as a code owner August 7, 2024 16:42
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Aug 7, 2024
Dreamsorcerer
Dreamsorcerer commented Aug 7, 2024
View reviewed changes
aiohttp/web_protocol.py Outdated
@@ -630,7 +630,6 @@ async def finish_response(
can get exception information. Returns True if the client disconnects
prematurely.
"""
request._finish()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@webknjaz Would be good if you can sanity check whether this method still makes sense. It was added only for this wait_for_disconnection() method, but then appears you added some multipart fix into the method. I'm wondering if that fix should live somewhere else, or we should keep the method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume your fix was never backported either: #5494.

@codecov Codecov
Copy link

codecov bot commented Aug 7, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.93%. Comparing base (e0ff524) to head (27feace).
Report is 855 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #8636      +/-   ##
==========================================
+ Coverage   97.90%   97.93%   +0.02%     
==========================================
  Files         107      107              
  Lines       33471    33458      -13     
  Branches     3938     3936       -2     
==========================================
- Hits        32769    32766       -3     
+ Misses        518      510       -8     
+ Partials      184      182       -2
Flag Coverage Δ
CI-GHA 97.84% <100.00%> (+0.02%) ⬆️
OS-Linux 97.50% <100.00%> (+0.02%) ⬆️
OS-Windows 95.88% <100.00%> (+0.02%) ⬆️
OS-macOS 97.17% <100.00%> (+0.02%) ⬆️
Py-3.10.11 97.32% <100.00%> (+0.02%) ⬆️
Py-3.10.14 97.25% <100.00%> (+0.02%) ⬆️
Py-3.11.9 97.48% <100.00%> (+0.02%) ⬆️
Py-3.12.4 97.61% <100.00%> (+0.02%) ⬆️
Py-3.8.10 95.64% <100.00%> (+0.02%) ⬆️
Py-3.8.18 97.15% <100.00%> (+0.02%) ⬆️
Py-3.9.13 97.31% <100.00%> (+0.02%) ⬆️
Py-3.9.19 97.25% <100.00%> (+0.02%) ⬆️
Py-pypy7.3.16 96.84% <100.00%> (+0.03%) ⬆️
VM-macos 97.17% <100.00%> (+0.02%) ⬆️
VM-ubuntu 97.50% <100.00%> (+0.02%) ⬆️
VM-windows 95.88% <100.00%> (+0.02%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Dreamsorcerer Dreamsorcerer added this to the 3.10.2 milestone Aug 7, 2024
@bdraco
Copy link
Member

bdraco commented Aug 8, 2024

I'm in the process of preparing .2 so I'll merge this one shortly unless there are any objections.

@bdraco bdraco merged commit 51d872e into master Aug 8, 2024
37 of 38 checks passed
@bdraco bdraco deleted the Dreamsorcerer-patch-4 branch August 8, 2024 14:56
@patchback Patchback
Copy link
Contributor

patchback bot commented Aug 8, 2024 •
edited
Loading

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

❌ Failed to cleanly apply 51d872e on top of patchback/backports/3.10/51d872e1b87480b84a1a541af394a34b311ec22c/pr-8636

Backporting merged PR #8636 into master

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. 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
  3. 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.10/51d872e1b87480b84a1a541af394a34b311ec22c/pr-8636 upstream/3.10
  4. Now, cherry-pick PR Remove Request.wait_for_disconnection() method #8636 contents into that branch: 
    $ git cherry-pick -x 51d872e1b87480b84a1a541af394a34b311ec22c
    If it'll yell at you with something like fatal: Commit 51d872e1b87480b84a1a541af394a34b311ec22c is a merge but no -m option was given., add -m 1 as follows instead: 
    $ git cherry-pick -m1 -x 51d872e1b87480b84a1a541af394a34b311ec22c
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR Remove Request.wait_for_disconnection() method #8636 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub: 
    $ git push origin patchback/backports/3.10/51d872e1b87480b84a1a541af394a34b311ec22c/pr-8636
  7. 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.

@patchback Patchback
Copy link
Contributor

patchback bot commented Aug 8, 2024 •
edited
Loading

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

❌ Failed to cleanly apply 51d872e on top of patchback/backports/3.11/51d872e1b87480b84a1a541af394a34b311ec22c/pr-8636

Backporting merged PR #8636 into master

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. 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
  3. 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.11/51d872e1b87480b84a1a541af394a34b311ec22c/pr-8636 upstream/3.11
  4. Now, cherry-pick PR Remove Request.wait_for_disconnection() method #8636 contents into that branch: 
    $ git cherry-pick -x 51d872e1b87480b84a1a541af394a34b311ec22c
    If it'll yell at you with something like fatal: Commit 51d872e1b87480b84a1a541af394a34b311ec22c is a merge but no -m option was given., add -m 1 as follows instead: 
    $ git cherry-pick -m1 -x 51d872e1b87480b84a1a541af394a34b311ec22c
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR Remove Request.wait_for_disconnection() method #8636 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub: 
    $ git push origin patchback/backports/3.11/51d872e1b87480b84a1a541af394a34b311ec22c/pr-8636
  7. 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.

bdraco pushed a commit that referenced this pull request Aug 8, 2024
@Dreamsorcerer @bdraco
Remove Request.wait_for_disconnection() method (#8636)
69150e4 
(cherry picked from commit 51d872e)
bdraco pushed a commit that referenced this pull request Aug 8, 2024
@Dreamsorcerer @bdraco
Remove Request.wait_for_disconnection() method (#8636)
287e0b5 
(cherry picked from commit 51d872e)
bdraco added a commit that referenced this pull request Aug 8, 2024
@bdraco @Dreamsorcerer
[PR #8636/51d872e backport][3.10] Remove Request.wait_for_disconnecti…
6a77806 
…on() method (#8650)

Co-authored-by: Sam Bull <git@sambull.org>
bdraco added a commit that referenced this pull request Aug 8, 2024
@bdraco @Dreamsorcerer
[PR #8636/51d872e backport][3.11] Remove Request.wait_for_disconnecti…
0de1c26 
…on() method (#8651)

Co-authored-by: Sam Bull <git@sambull.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bdraco bdraco bdraco approved these changes

@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner

@webknjaz webknjaz Awaiting requested review from webknjaz webknjaz is a code owner

Assignees
No one assigned
Labels
backport-3.11 Trigger automatic backporting to the 3.11 release branch by Patchback robot bot:chronographer:provided There is a change note present in this PR
Projects
None yet
Milestone
3.10.2
Development

Successfully merging this pull request may close these issues.

Remove wait_for_disconnection
2 participants
@Dreamsorcerer @bdraco