Skip to content

gh-101143: Remove references to TimerHandle from asyncio.base_events.BaseEventLoop._add_callback#101197

Merged
kumaraditya303 merged 5 commits intopython:mainfrom
bdraco:_add_callback
Jan 21, 2023
Merged

gh-101143: Remove references to TimerHandle from asyncio.base_events.BaseEventLoop._add_callback#101197
kumaraditya303 merged 5 commits intopython:mainfrom
bdraco:_add_callback

Conversation

@bdraco
Copy link
Contributor

@bdraco bdraco commented Jan 20, 2023 •
edited
Loading

gh-101143: Remove references to TimerHandle from asyncio.base_events.BaseEventLoop._add_callback

After 592ada9 it appears this function no longer cares about self._scheduled or TimerHandle but the docstring seems to imply otherwise

fixes #101143

bdraco added 2 commits January 20, 2023 10:34
@bdraco
pythongh-101143: Remove references to TimerHandle from asyncio.base_e…
cb2f8d6 
…vents.BaseEventLoop._add_callback

After 592ada9 this function never adds to _scheduled
@bdraco
add NEWS
5218551
@bedevere-bot bedevere-bot mentioned this pull request Jan 20, 2023
Closed
@ghost
Copy link

ghost commented Jan 20, 2023 •
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bdraco

This comment was marked as outdated.

Sign in to view
gvanrossum
gvanrossum reviewed Jan 20, 2023
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

I'm guessing that since you're deleting only dead code this doesn't need a new test.

gvanrossum
gvanrossum approved these changes Jan 20, 2023
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Works for me. I'd like @kumaraditya303 to take a look first though. Kumar, if you approve just merge it. Also, should we backport this to 3.11? Or even 3.10? The change that made it all pointless is pretty old IIUC.

@bdraco
Copy link
Contributor Author

bdraco commented Jan 20, 2023

Also, should we backport this to 3.11? Or even 3.10? The change that made it all pointless is pretty old IIUC.

Backport to 3.10 (where I saw this in production)/3.11 would be great since this has a real world impact for Home Assistant users processing d-bus messages especially on the slower RPis.

@kumaraditya303
Copy link
Contributor

kumaraditya303 commented Jan 21, 2023

Also, should we backport this to 3.11? Or even 3.10? The change that made it all pointless is pretty old IIUC.

Let's backport it.

@kumaraditya303 kumaraditya303 merged commit 9e94767 into python:main Jan 21, 2023
@miss-islington
Copy link
Contributor

miss-islington commented Jan 21, 2023

Thanks @bdraco for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

miss-islington commented Jan 21, 2023

Sorry @bdraco and @kumaraditya303, I had trouble checking out the 3.11 backport branch.
Please retry by removing and re-adding the "needs backport to 3.11" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 9e947675ae3dc32f5863e5ed3022301cf7fd79b4 3.11

@kumaraditya303 kumaraditya303 removed the needs backport to 3.11 only security fixes label Jan 21, 2023
@kumaraditya303 kumaraditya303 added the needs backport to 3.11 only security fixes label Jan 21, 2023
@miss-islington
Copy link
Contributor

miss-islington commented Jan 21, 2023

Thanks @bdraco for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 21, 2023
@bdraco @kumaraditya303 @miss-islington
pythongh-101143: Remove references to TimerHandle from `asyncio.bas…
c43548f 
…e_events.BaseEventLoop._add_callback` (pythonGH-101197)

(cherry picked from commit 9e94767)

Co-authored-by: J. Nick Koston <nick@koston.org>
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
@bedevere-bot
Copy link

bedevere-bot commented Jan 21, 2023

GH-101216 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jan 21, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 21, 2023
@bdraco @kumaraditya303 @miss-islington
pythongh-101143: Remove references to TimerHandle from `asyncio.bas…
4484f7d 
…e_events.BaseEventLoop._add_callback` (pythonGH-101197)

(cherry picked from commit 9e94767)

Co-authored-by: J. Nick Koston <nick@koston.org>
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
@bedevere-bot
Copy link

bedevere-bot commented Jan 21, 2023

GH-101217 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jan 21, 2023
miss-islington added a commit that referenced this pull request Jan 21, 2023
@miss-islington @bdraco @kumaraditya303
gh-101143: Remove references to TimerHandle from `asyncio.base_even…
99da75e 
…ts.BaseEventLoop._add_callback` (GH-101197)

(cherry picked from commit 9e94767)

Co-authored-by: J. Nick Koston <nick@koston.org>
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
miss-islington added a commit that referenced this pull request Jan 21, 2023
@miss-islington @bdraco @kumaraditya303
gh-101143: Remove references to TimerHandle from `asyncio.base_even…
9532c04 
…ts.BaseEventLoop._add_callback` (GH-101197)

(cherry picked from commit 9e94767)

Co-authored-by: J. Nick Koston <nick@koston.org>
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
@bdraco
Copy link
Contributor Author

bdraco commented Jan 21, 2023

Thanks for taking the time to review this.

@gvanrossum
Copy link
Member

gvanrossum commented Jan 21, 2023

Thanks for the PR! It’s always great to delete code, we need more of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes

@gvanrossum gvanrossum gvanrossum approved these changes

@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner

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

Assignees

@kumaraditya303 kumaraditya303

Labels

topic-asyncio

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

asyncio.base_events.BaseEventLoop._add_callback references TimerHandle but never uses them

5 participants

@bdraco @kumaraditya303 @miss-islington @bedevere-bot @gvanrossum