Rework how Guild member syncing works, deprecate `Guild.sync`

This commit cleans up a mess I created when I originally added the
`Guild.sync` functionality. The original intention of this function
was to provide a simple interface to ensuring the local state of a
guild was "up to date". This abstraction turned out to be a bad idea
for the following reasons;

- Its completely unclear what "sync" means in the context of a Guild.
Hiding the way syncing guild members works (which is a pretty "developer
unfriendly" as it is) just results in more confusion for very little
value.
- Discord technically supports sending multiple guild ids in a single
`REQUEST_GUILD_MEMBERS` op which this interface didn't support or
handle. Using this function as it was intended on a largeish bot would
result in your gateway client getting rate limited for quite a bit on
startup (which is lame).
- This implementation of guild member syncing resulted in a `synced`
flag on guilds, which is basically completely useless and doesn't mean
anything much at all (since REQUEST_GUILD_MEMBERS is purely async and
there is no correct way to confirm a request was fulfilled.

To cleanup this mess I've made the following changes;

- Entirely deprecate `Guild.sync`. This is just a bad function name and
should die in a fire to save our future children from pain and
frustration.
- Add `Guild.request_guild_members`. This function now properly conveys
what it does and provides a more correct interface to this Discord
functionality (nb: Guild.sync now calls this function, which does not
prevent multiple calls from firing requests).
- Add `GatewayClient.request_guild_members`. This function provides an
alternative interface for sending the request guild members op without
low-level GatewayClient interfacing. Ideally users who need to avoid
being rate limited and/or want better guild member syncing performance
can use this function which accepts multiple guild ids.

TL;DR;

- If you don't call `Guild.sync`; you don't need to worry about
anything.
- If you call `Guild.sync` anywhere at all and you don't really care
about the performance impact of syncing guild members one-by-one; just
change your code to call `Guild.request_guild_members`.
- If you call `Guild.sync` in a `GUILD_CREATE` handler or you care about
the performance impact of syncing multiple Guilds; move to using the
underlying GatewayClient interface and sync multiple guilds at once
(likely in batches).

disco/gateway/client.py

@@ -263,3 +263,15 @@ class GatewayClient(LoggingClass):
     def run(self):
         gevent.spawn(self.connect_and_run)
         self.ws_event.wait()
+
+    def request_guild_members(self, guild_id_or_ids, query=None, limit=0):
+        """
+        Request a batch of Guild members from Discord. Generally this function
+        can be called when initially loading Guilds to fill the local member state.
+        """
+        self.send(OPCode.REQUEST_GUILD_MEMBERS, {
+            # This is simply unfortunate naming on the part of Discord...
+            'guild_id': guild_id_or_ids,
+            'query': query or '',
+            'limit': limit,
+        })

disco/state.py

@@ -51,7 +51,9 @@ class StateConfig(Config):
     sync_guild_members : bool
         If true, guilds will be automatically synced when they are initially loaded
         or joined. Generally this setting is OK for smaller bots, however bots in over
-        50 guilds will notice this operation can take a while to complete.
+        50 guilds will notice this operation can take a while to complete and may want
+        to batch requests using the underlying `GatewayClient.request_guild_members`
+        interface.
     """
     track_messages = True
     track_messages_size = 100
@@ -200,7 +202,7 @@ class State(object):
             self.voice_states[voice_state.session_id] = voice_state
 
         if self.config.sync_guild_members:
-            event.guild.sync()
+            event.guild.request_guild_members()
 
     def on_guild_update(self, event):
         self.guilds[event.guild.id].inplace_update(event.guild, ignored=[

disco/types/guild.py

@@ -3,7 +3,6 @@ import warnings
 
 from holster.enum import Enum
 
-from disco.gateway.packets import OPCode
 from disco.api.http import APIException
 from disco.util.paginator import Paginator
 from disco.util.snowflake import to_snowflake
@@ -326,8 +325,6 @@ class Guild(SlottedModel, Permissible):
     voice_states = AutoDictField(VoiceState, 'session_id')
     member_count = Field(int)
 
-    synced = Field(bool, default=False)
-
     def __init__(self, *args, **kwargs):
         super(Guild, self).__init__(*args, **kwargs)
 
@@ -424,16 +421,15 @@ class Guild(SlottedModel, Permissible):
 
         return self.client.api.guilds_roles_modify(self.id, to_snowflake(role), **kwargs)
 
+    def request_guild_members(self, query=None, limit=0):
+        self.client.gw.request_guild_members(self.id, query, limit)
+
     def sync(self):
-        if self.synced:
-            return
-
-        self.synced = True
-        self.client.gw.send(OPCode.REQUEST_GUILD_MEMBERS, {
-            'guild_id': self.id,
-            'query': '',
-            'limit': 0,
-        })
+        warnings.warn(
+            'Guild.sync has been deprecated in place of Guild.request_guild_members',
+            DeprecationWarning)
+
+        self.request_guild_members()
 
     def get_bans(self):
         return self.client.api.guilds_bans_list(self.id)