3

Background

Hi everyone. I've used the discord.py package to write a Discord bot for a server that I own and manage. The server is an unofficially official community for a retail brand in the United States and is dominated by employees. We have a role (@U.S. Employee) to distinguish employees and grant access to employee-only channels. Once a year, we require employees to verify that they're still an employee. Once verified, they're given the @Verified - 2022 role.

I've written a command called prune_unverified where it will iterate over all server members and checks whether or not they have either of these roles.

Here's how the command works: If the server member doesn't have the @U.S. Employee role (or the @Canada Employee role), then don't do anything. If the employee has either of these employee roles and also has the @Verified - 2022 role, then also don't do anything. However, if the employee has either employee role and does not have the @Verified - 2022 role, then all of their roles are removed and replaced with the @Former Employee role.

The Problem

The code that I've written runs and executes flawlessly with zero errors during my small scale tests. It works exactly as expected. The issue is that every year when running the command for real, the number of server members who have either employee role but does not have the verified role (and thus will be given the former employee role) is well over 1,000 server members.

I'll run the command, it will work perfectly for less than 200 server members and then it will just stop. The bot is supposed to reply with a message once it completes, but it never does. The bot doesn't crash though - it's literally just this one command will stop working. Even trying it again won't work anymore. Even rebooting the bot won't work. All other functions of the bot continues to work perfectly fine.

The source code of the bot is hosted in a private GitHub repository which is synced with a Heroku app on the heroku-20 stack with automated deployments enabled. It's slug size right now is 62.9 MiB of 500 MiB. It's also using hobby dynos - the one that only costs $7/dyno/month.

The code for the command I'm talking about is below. So how can I write this code better so that it works actioning upon over 1,000 server members at once? Am I being limited by the Discord API? Or the discord.py framework? Do I not have enough memory or performance on the Heroku hobby dyno? Any help would be greatly appreciated. I really would like to get this code to work but I'm not the greatest programmer and I'm not sure how else I can write this code.

@bot.command(name = "prune_unverified", help = "prune the unverified employees", enabled = True)
@commands.has_role("Owner")
async def prune_unverified(context):
    await context.message.delete()
    guild = discord.utils.get(bot.guilds, name = GUILD)

    ServerMembers     = [i for i in guild.members]
    VerifiedEmployees = []
    PrunedUsers       = []

    EmployeeRoleUS = guild.get_role(EMPLOYEE_ROLE)
    EmployeeRoleCA = guild.get_role(EMPLOYEE_CA_ROLE)
    VerifiedRole   = guild.get_role(EMPLOYEE_VERIFIED_ROLE)

    for user in ServerMembers:
        if (EmployeeRoleUS or EmployeeRoleCA) in user.roles:
            if VerifiedRole in user.roles:
                VerifiedEmployees.append(user)
            else:
                PrunedUsers.append(user)
        else:
            continue

    for user in PrunedUsers:
        await guild.get_member(user.id).edit(roles = []) # Remove all roles
        await guild.get_member(user.id).add_roles(guild.get_role(EMPLOYEE_FORMER_ROLE)) # Award the former role
        await asyncio.sleep(15)
        # pass # Adding a pass here because we are just testing

    # create a .csv file of pruned users
    with open("pruned_users.csv", mode = "w") as pu_file:
        pu_writer = csv.writer(pu_file, delimiter = ",")
        pu_writer.writerow(["Server Nickname", "Username", "ID"])
        for user in PrunedUsers:
            pu_writer.writerow([f"{user.nick}", f"{user.name}#{user.discriminator}", f"{user.id}"])

    # create a .csv file of verified users
    with open("verified_users.csv", mode = "w") as vu_file:
        vu_writer = csv.writer(vu_file, delimiter = ",")
        vu_writer.writerow(["Server Nickname", "Username", "ID"])
        for user in VerifiedEmployees:
            vu_writer.writerow([f"{user.nick}", f"{user.name}#{user.discriminator}", f"{user.id}"])

    embed = discord.Embed(
        description = f":crossed_swords: **`{len(PrunedUsers)}` users were pruned by <@{context.author.id}>.**"
                      + f"\n:shield: **There are `{len(VerifiedEmployees)}` users who completed re-verification.**"
                      + f"\n\n**Here is a `.csv` file of all the users that were pruned:**",
        color = discord.Color.blue()
    )
    await context.send(embed = embed)
    await context.send(file = discord.File(r"pruned_users.csv")) # upload the .csv file we created
    await context.send(file = discord.File(r"verified_users.csv"))
    await log_command(context, "prune_unverified")

UPDATE: Sunday June 26, 2022 @ ~4:13 PM EST

Thanks to your help, I've rewritten the logic and also changed how I retrieve all server members. After running this new code, it took the command approximately 24 minutes to execute and it worked flawlessly. Thank you everybody! :D

@bot.command(name = "prune_unverified", help = "prune the unverified employees", enabled = True)
@commands.has_role("Owner")
async def prune_unverified(context):
    await context.message.delete()

    VerifiedEmployees = []
    PrunedUsers       = []

    EmployeeRoleUS = context.guild.get_role(EMPLOYEE_ROLE)
    EmployeeRoleCA = context.guild.get_role(EMPLOYEE_CA_ROLE)
    VerifiedRole   = context.guild.get_role(EMPLOYEE_VERIFIED_ROLE)

    # Begin new code here
    async for user in context.guild.fetch_members(limit = None):
        if (EmployeeRoleUS in user.roles) or (EmployeeRoleCA in user.roles):
            if VerifiedRole in user.roles:
                VerifiedEmployees.append(user)
            else:
                PrunedUsers.append(user)
        else:
            continue
    # End new code here

    for user in PrunedUsers:
        #await user.edit(roles = []) # Remove all roles (Commented out for testing)
        await user.add_roles(context.guild.get_role(990703166041501806)) # Award the former role

    # create a .csv file of pruned users
    with open("pruned_users.csv", mode = "w") as pu_file:
        pu_writer = csv.writer(pu_file, delimiter = ",")
        pu_writer.writerow(["Server Nickname", "Username", "ID"])
        for user in PrunedUsers:
            pu_writer.writerow([f"{user.nick}", f"{user.name}#{user.discriminator}", f"{user.id}"])

    # create a .csv file of verified users
    with open("verified_users.csv", mode = "w") as vu_file:
        vu_writer = csv.writer(vu_file, delimiter = ",")
        vu_writer.writerow(["Server Nickname", "Username", "ID"])
        for user in VerifiedEmployees:
            vu_writer.writerow([f"{user.nick}", f"{user.name}#{user.discriminator}", f"{user.id}"])

    embed = discord.Embed(
        description = f":crossed_swords: **`{len(PrunedUsers)}` users were pruned by <@{context.author.id}>.**"
                      + f"\n:shield: **There are `{len(VerifiedEmployees)}` users who completed re-verification.**"
                      + f"\n\n**Here is a `.csv` file of all the users that were pruned:**",
        color = discord.Color.blue()
    )
    await context.send(embed = embed)
    await context.send(file = discord.File(r"pruned_users.csv")) # upload the .csv file we created
    await context.send(file = discord.File(r"verified_users.csv"))
    await log_command(context, "prune_unverified")
  • 1
    How long did it take for it to finish 200 users? Have you tried lowering the `await asyncio.sleep(15)` to `0`? – Queuebee Jun 16 '22 at 19:55
  • @Queuebee I believe it took around an hour. Wouldn't you think lowering the `await asyncio.sleep(15)` to `0` have the opposite effect? The reason why I added the sleep is because I was worried of hitting a rate limit – Iron_Legion Jun 16 '22 at 21:04
  • 1
    I wasn't aware of the rate limit, good call. So have you waited over 4 hours (15*1000+/3600) when you tested it for over 1000 users? – Queuebee Jun 16 '22 at 21:12
  • @Queuebee I didn't wait a full 4 hours, however I could tell in my server's audit log and MEE6 log that the bot had stopped removing roles and the number of users with the `@U.S. Employee` role hadn't gone down nearly to the number I would have expected it to be – Iron_Legion Jun 16 '22 at 21:37
  • 1
    Rate limit for REST: PATCH Member action is 10/10 seconds per guild so in theory you can lower down your 15 seconds. But unfortunately, I don't see anything too wrong with the code except some unnecessary stuff like `= [i for i in guild.members]` instead of just `= guild.members` or `for user in PrunedUsers:` and then you do `.get_member` while your `user` variable is already `discord.Member` object. Or not having variable for `guild.get_role(EMPLOYEE_FORMER_ROLE)` to use in loop like you do for `VerifiedRole ` – Aluerie Jun 16 '22 at 22:23
  • 2
    Are you sure `if (EmployeeRoleUS or EmployeeRoleCA) in user.roles:` works as intended? I don't have a quick way to test this, but if `bool(EmployeeRoleUS)` returns `True`, that line will always fire thus all users will either be pruned or verified. This might make the PrunedUsers loop way larger than you expected, and it could cause 'pruning' of users that can't be pruned. See [this](https://stackoverflow.com/questions/21344842/if-a-or-b-in-l-where-l-is-a-list-python) – Queuebee Jun 16 '22 at 23:03
  • 2
    It may be caching. Using `fetch` won't actually give you the members ([source](https://discordpy.readthedocs.io/en/latest/api.html?highlight=fetch_guild#discord.Client.fetch_guild)), so you'll have to rely on the fact that the bot knows all the members. Also `[i for i in guild.members]` is unnecessary - the return type is already a list. It's also a bad idea to cycle between getting members with the ids, you should add the member object once and then use it instead of converting back and forth. – Eric Jin Jun 16 '22 at 23:51
  • @Queuebee Sorry for the delayed response. I was under the impression that conditional worked because it worked in small scale tests. But after reading that thread you linked, I think my logic there is flawed and needs some tweaking. That could be part of the reason why it just stops. Thank you! In `EmployeeRoleUS`, the `EMPLOYEE_ROLE` variable is the Role ID for the `@U.S. Employee` role – Iron_Legion Jun 19 '22 at 16:44
  • @EricJin I agree, the list comprehension there is unnecessary - I don't remember why I did that instead of `guild.members`. When you say `fetch`, are you talking about line 5 when I do `discord.utils.get()`? If so, how do I get the list of all server members then? Again, forgive my denseness, where should I add that member object? – Iron_Legion Jun 19 '22 at 16:49
  • 2
    When you do `[VerifiedEmployees/PrunedUsers].append(user)`, you already have the member object. Spare yourself some suffering and simply do `user.edit`, `user.add_roles`, etc. Also btw, with the ratelimits, discord will handle them automatically. It will wait for the ratelimit to go out before sending another request (you can see this in action if you try to send 20 messages in a row, when it will send them in batches of 5 and then wait for the ratelimit to go up), so you really don't need that. – Eric Jin Jun 19 '22 at 18:09
  • @EricJin Ah I see. That makes sense, thank you. How should I go about obtaining all of the server members though if what I'm doing now isn't? I feel like that might be my biggest problem and why the bot stops after less than 200. – Iron_Legion Jun 21 '22 at 02:53
  • 1
    `guild.members` should be working, but it might not be cached. Is there a need to use `utils.get`, or can you just use `ctx.guild`? – Eric Jin Jun 21 '22 at 12:32
  • @EricJin Using `utils.get` was just an easy way for me to fetch the guild's methods and properties, I have it at the beginning of every command. I didn't realize that I could just get that with the `ctx` though. So theoretically then, iterating over `ctx.guild.members` should not be cached and actually get me every server member? – Iron_Legion Jun 21 '22 at 15:16
  • 1
    Yes, if you're running from the guild. Can you [edit] the question with your updated code so we can try to find the problem with it? – Eric Jin Jun 21 '22 at 20:33
  • @EricJin and Queuebee I've just updated my question with my updated code. I was able to rewrite the conditional to be more accurate (thanks to [this question](https://stackoverflow.com/questions/21344842/if-a-or-b-in-l-where-l-is-a-list-python) that Queuebee linked. I was also able to change how I retrieve the server members and now my code is actually retrieving every single server member. It took about 24 minutes to run the updated command. – Iron_Legion Jun 26 '22 at 20:21
  • 1
    That sounds pretty good, only 1-2 seconds per member which is definitely well within the ratelimit region. I doubt you'll ever get too much faster. I hope we've been helpful :) – Eric Jin Jun 26 '22 at 21:02
  • @Iron_Legion you can use the edit as an answer to your own question – Queuebee Jul 09 '22 at 00:16

0 Answers0