Skip to content

Make .sleep(..) return a coroutine#18772

Merged
erikjohnston merged 4 commits intodevelopfrom
erikj/lint_sleep
Aug 5, 2025
Merged

Make .sleep(..) return a coroutine#18772
erikjohnston merged 4 commits intodevelopfrom
erikj/lint_sleep

Conversation

@erikjohnston
Copy link
Copy Markdown
Member

This helps ensure that mypy can catch places where we don't await on it, like in #18763.

This helps ensure that mypy can catch places where we don't await on it,
like in #18763.
@erikjohnston erikjohnston marked this pull request as ready for review August 4, 2025 12:47
@erikjohnston erikjohnston requested a review from a team as a code owner August 4, 2025 12:47
Comment thread synapse/util/__init__.py
Comment thread changelog.d/18772.misc Outdated
Comment thread synapse/util/__init__.py

@defer.inlineCallbacks
def sleep(self, seconds: float) -> "Generator[Deferred[float], Any, Any]":
async def sleep(self, seconds: float) -> None:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like this other spot doesn't have types and is in contrib/ so we don't necessarily need to change it but we could also align

def sleep(self, seconds):
d = defer.Deferred()
reactor.callLater(seconds, d.callback, seconds)
return d

Copy link
Copy Markdown
Member Author

@erikjohnston erikjohnston Aug 4, 2025

Choose a reason for hiding this comment

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

Oh, mmm good spot. Looking at it though I'm disinclined to try and move it to async/await as it still users generators and @defer.inlineCallbacks everywhere else. If anything I'd be inclined to remove the cmdclient package entirely

Co-authored-by: Eric Eastwood <erice@element.io>
@erikjohnston erikjohnston merged commit 2061511 into develop Aug 5, 2025
44 checks passed
@erikjohnston erikjohnston deleted the erikj/lint_sleep branch August 5, 2025 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants