-
-
Notifications
You must be signed in to change notification settings - Fork 478
refactor: Update Client.run
to have a better async I/O usage
#2645
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.
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
Open
DA-344
wants to merge
30
commits into
Pycord-Development:master
Choose a base branch
from
DA-344:fix/client-run
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+89
−58
Open
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
de49cf5
fix: client.run not allowing bots to start
DA-344 4fe559f
chore: Update more things
DA-344 74863c5
chore: Move the loop update to .start
DA-344 72f904c
chore: Added logging and updated `run` to specify the arguments
DA-344 7e7f1e9
style(pre-commit): auto fixes from pre-commit.com hooks
pre-commit-ci[bot] f289f74
chore: Updated CHANGELOG
DA-344 2195dc6
Merge changes from remote
DA-344 3d235ee
style(pre-commit): auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 07ccbc9
dot
DA-344 ef835dc
Merge branch 'master' into fix/client-run
Lulalaby 5e8a322
Update docstrings
DA-344 de5156c
Update docstrings
DA-344 b830a94
merge 'master' from https://github.com/Pycord-Development/pycord
DA-344 88484e2
chore: Update Client.__aenter__ and Client.run
DA-344 ba81ebe
feat: Add operations container to Client docstring
DA-344 f5f0085
chore: merge branch 'master' from Pycord-Development/pycord
DA-344 28fab35
chore: Update Client.close to prevent double closing and race conditions
DA-344 c378d41
fix: Indentation error
DA-344 e818a1c
Merge branch 'master' into fix/client-run
Dorukyum e963341
chore: Update Client.close and Client.clear to correctly update and u…
DA-344 e19e029
Merge branch 'master' into fix/client-run
DA-344 abf0ca0
Merge branch 'master' of https://github.com/Pycord-Development/pycord…
DA-344 81541a3
Merge branch 'master' into fix/client-run
DA-344 2735849
Merge branch 'master' into fix/client-run
DA-344 c88cac8
merge master
DA-344 0bc5014
chore: make loop a property
DA-344 e60bda3
style(pre-commit): auto fixes from pre-commit.com hooks
pre-commit-ci[bot] cb05d05
chore: make cleanup be done only when necessary
DA-344 81e61b8
Merge branch 'fix/client-run' of https://github.com/DA-344/pycord int…
DA-344 3f0783a
change from = MISSING to = None
DA-344 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,7 +27,6 @@ | |
|
||
import asyncio | ||
import logging | ||
import signal | ||
import sys | ||
import traceback | ||
from types import TracebackType | ||
|
@@ -122,6 +121,12 @@ class Client: | |
|
||
A number of options can be passed to the :class:`Client`. | ||
|
||
.. container:: operations | ||
|
||
.. describe:: async with x | ||
|
||
Asynchronously initializes the client. | ||
|
||
Parameters | ||
----------- | ||
max_messages: Optional[:class:`int`] | ||
|
@@ -226,9 +231,7 @@ def __init__( | |
): | ||
# self.ws is set in the connect method | ||
self.ws: DiscordWebSocket = None # type: ignore | ||
self.loop: asyncio.AbstractEventLoop = ( | ||
asyncio.get_event_loop() if loop is None else loop | ||
) | ||
self._loop: asyncio.AbstractEventLoop | None = loop | ||
self._listeners: dict[str, list[tuple[asyncio.Future, Callable[..., bool]]]] = ( | ||
{} | ||
) | ||
|
@@ -256,7 +259,8 @@ def __init__( | |
self._enable_debug_events: bool = options.pop("enable_debug_events", False) | ||
self._connection: ConnectionState = self._get_state(**options) | ||
self._connection.shard_count = self.shard_count | ||
self._closed: bool = False | ||
self._closed: asyncio.Event = asyncio.Event() | ||
self._closing_task: asyncio.Lock = asyncio.Lock() | ||
self._ready: asyncio.Event = asyncio.Event() | ||
self._connection._get_websocket = self._get_websocket | ||
self._connection._get_client = lambda: self | ||
|
@@ -270,12 +274,23 @@ def __init__( | |
self._tasks = set() | ||
|
||
async def __aenter__(self) -> Client: | ||
loop = asyncio.get_running_loop() | ||
self.loop = loop | ||
self.http.loop = loop | ||
self._connection.loop = loop | ||
if self._loop is MISSING: | ||
try: | ||
self._loop = asyncio.get_running_loop() | ||
except RuntimeError: | ||
# No event loop was found, this should not happen | ||
DA-344 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# because entering on this context manager means a | ||
# loop is already active, but we need to handle it | ||
# anyways just to prevent future errors. | ||
|
||
# Maybe handle different system event loop policies? | ||
self._loop = asyncio.new_event_loop() | ||
|
||
self.http.loop = self.loop | ||
self._connection.loop = self.loop | ||
|
||
self._ready = asyncio.Event() | ||
self._closed = asyncio.Event() | ||
|
||
return self | ||
|
||
|
@@ -308,6 +323,21 @@ def _get_state(self, **options: Any) -> ConnectionState: | |
def _handle_ready(self) -> None: | ||
self._ready.set() | ||
|
||
@property | ||
def loop(self) -> asyncio.AbstractEventLoop: | ||
"""The event loop that the client uses for asynchronous operations.""" | ||
if self._loop is None: | ||
raise RuntimeError("loop is not set") | ||
return self._loop | ||
|
||
@loop.setter | ||
def loop(self, value: asyncio.AbstractEventLoop) -> None: | ||
if not isinstance(value, asyncio.AbstractEventLoop): | ||
raise TypeError( | ||
f"expected a AbstractEventLoop object, got {value.__class__.__name__!r} instead" | ||
) | ||
self._loop = value | ||
|
||
@property | ||
def latency(self) -> float: | ||
"""Measures latency between a HEARTBEAT and a HEARTBEAT_ACK in seconds. If no websocket | ||
|
@@ -712,23 +742,24 @@ async def close(self) -> None: | |
|
||
Closes the connection to Discord. | ||
""" | ||
if self._closed: | ||
return | ||
async with self._closing_task: | ||
if self.is_closed(): | ||
return | ||
|
||
await self.http.close() | ||
self._closed = True | ||
await self.http.close() | ||
|
||
for voice in self.voice_clients: | ||
try: | ||
await voice.disconnect(force=True) | ||
except Exception: | ||
# if an error happens during disconnects, disregard it. | ||
pass | ||
for voice in self.voice_clients: | ||
try: | ||
await voice.disconnect(force=True) | ||
except Exception: | ||
# if an error happens during disconnects, disregard it. | ||
pass | ||
|
||
if self.ws is not None and self.ws.open: | ||
await self.ws.close(code=1000) | ||
if self.ws is not None and self.ws.open: | ||
await self.ws.close(code=1000) | ||
|
||
self._ready.clear() | ||
self._ready.clear() | ||
self._closed.set() | ||
|
||
def clear(self) -> None: | ||
"""Clears the internal state of the bot. | ||
|
@@ -737,7 +768,7 @@ def clear(self) -> None: | |
and :meth:`is_ready` both return ``False`` along with the bot's internal | ||
cache cleared. | ||
""" | ||
self._closed = False | ||
self._closed.clear() | ||
self._ready.clear() | ||
self._connection.clear() | ||
self.http.recreate() | ||
|
@@ -752,10 +783,11 @@ async def start(self, token: str, *, reconnect: bool = True) -> None: | |
TypeError | ||
An unexpected keyword argument was received. | ||
""" | ||
# Update the loop to get the running one in case the one set is MISSING | ||
DA-344 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
await self.login(token) | ||
await self.connect(reconnect=reconnect) | ||
|
||
def run(self, *args: Any, **kwargs: Any) -> None: | ||
def run(self, token: str, *, reconnect: bool = True) -> None: | ||
"""A blocking call that abstracts away the event loop | ||
initialisation from you. | ||
|
||
|
@@ -766,60 +798,57 @@ def run(self, *args: Any, **kwargs: Any) -> None: | |
Roughly Equivalent to: :: | ||
|
||
try: | ||
loop.run_until_complete(start(*args, **kwargs)) | ||
asyncio.run(start(token)) | ||
except KeyboardInterrupt: | ||
loop.run_until_complete(close()) | ||
# cancel all tasks lingering | ||
finally: | ||
loop.close() | ||
return | ||
|
||
Parameters | ||
---------- | ||
token: :class:`str` | ||
The authentication token. Do not prefix this token with | ||
anything as the library will do it for you. | ||
reconnect: :class:`bool` | ||
If we should attempt reconnecting to the gateway, either due to internet | ||
failure or a specific failure on Discord's part. Certain | ||
disconnects that lead to bad state will not be handled (such as | ||
invalid sharding payloads or bad tokens). | ||
|
||
.. warning:: | ||
|
||
This function must be the last function to call due to the fact that it | ||
is blocking. That means that registration of events or anything being | ||
called after this function call will not execute until it returns. | ||
""" | ||
loop = self.loop | ||
|
||
try: | ||
loop.add_signal_handler(signal.SIGINT, loop.stop) | ||
loop.add_signal_handler(signal.SIGTERM, loop.stop) | ||
except (NotImplementedError, RuntimeError): | ||
pass | ||
|
||
async def runner(): | ||
try: | ||
await self.start(*args, **kwargs) | ||
finally: | ||
if not self.is_closed(): | ||
await self.close() | ||
async with self: | ||
await self.start(token=token, reconnect=reconnect) | ||
|
||
def stop_loop_on_completion(f): | ||
loop.stop() | ||
try: | ||
run = self.loop.run_until_complete | ||
requires_cleanup = True | ||
except RuntimeError: | ||
run = asyncio.run | ||
requires_cleanup = False | ||
|
||
future = asyncio.ensure_future(runner(), loop=loop) | ||
future.add_done_callback(stop_loop_on_completion) | ||
try: | ||
loop.run_forever() | ||
except KeyboardInterrupt: | ||
_log.info("Received signal to terminate bot and event loop.") | ||
run(runner()) | ||
finally: | ||
future.remove_done_callback(stop_loop_on_completion) | ||
_log.info("Cleaning up tasks.") | ||
_cleanup_loop(loop) | ||
# Ensure the bot is closed | ||
if not self.is_closed(): | ||
self.loop.run_until_complete(self.close()) | ||
|
||
if not future.cancelled(): | ||
try: | ||
return future.result() | ||
except KeyboardInterrupt: | ||
# I am unsure why this gets raised here but suppress it anyway | ||
return None | ||
# asyncio.run automatically does the cleanup tasks, so if we use | ||
# it we don't need to clean up the tasks. | ||
if requires_cleanup: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't you just do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems a little bit unclean |
||
_log.info("Cleaning up tasks.") | ||
_cleanup_loop(self.loop) | ||
|
||
# properties | ||
|
||
def is_closed(self) -> bool: | ||
"""Indicates if the WebSocket connection is closed.""" | ||
return self._closed | ||
return self._closed.is_set() | ||
|
||
@property | ||
def activity(self) -> ActivityTypes | None: | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Idk if this is the best wording, the scope of this pr seems to be broader than just fixing errors. But idk this is also probably good enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all this changes are mainly to fix errors due to asyncio so it is good as it is now ig