From ab0d44b8e013694c2e92af51c6fdb8ed9239c331 Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Wed, 25 Dec 2019 18:20:16 +0100 Subject: Hardcode SIGKILL value It allows the cog to also work on Windows, because of Signals.SIGKILL not being defined on this platform --- bot/cogs/snekbox.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index da33e27b2..e9e5465ad 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -36,6 +36,8 @@ RAW_CODE_REGEX = re.compile( MAX_PASTE_LEN = 1000 EVAL_ROLES = (Roles.helpers, Roles.moderator, Roles.admin, Roles.owner, Roles.rockstars, Roles.partners) +SIGKILL = 9 + class Snekbox(Cog): """Safe evaluation of Python code using Snekbox.""" @@ -101,7 +103,7 @@ class Snekbox(Cog): if returncode is None: msg = "Your eval job has failed" error = stdout.strip() - elif returncode == 128 + Signals.SIGKILL: + elif returncode == 128 + SIGKILL: msg = "Your eval job timed out or ran out of memory" elif returncode == 255: msg = "Your eval job has failed" -- cgit v1.2.3 From e53492ed9485e169b2fa471635c2ea624ec1d532 Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Thu, 26 Dec 2019 11:17:48 +0100 Subject: Correct eval output to include the 11th line --- bot/cogs/snekbox.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index e9e5465ad..00b8618e2 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -154,7 +154,7 @@ class Snekbox(Cog): lines = output.count("\n") if lines > 0: - output = output.split("\n")[:10] # Only first 10 cause the rest is truncated anyway + output = output.split("\n")[:11] # Only first 11 cause the rest is truncated anyway output = (f"{i:03d} | {line}" for i, line in enumerate(output, 1)) output = "\n".join(output) -- cgit v1.2.3 From b5730e0b07a4eb886710d71648ba4c0ffb4ebf79 Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Tue, 28 Jan 2020 14:50:58 +0000 Subject: Don't strip whitespaces during snekbox formatting It could lead to a misleading result if it is stripped. --- bot/cogs/snekbox.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 00b8618e2..81951efd3 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -137,7 +137,7 @@ class Snekbox(Cog): """ log.trace("Formatting output...") - output = output.strip(" \n") + output = output.rstrip("\n") original_output = output # To be uploaded to a pasting service if needed paste_link = None @@ -171,7 +171,6 @@ class Snekbox(Cog): if truncated: paste_link = await self.upload_output(original_output) - output = output.strip() if not output: output = "[No output]" -- cgit v1.2.3 From 6991e5fab13893c05d6f220e71f6ffc71509c1aa Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Wed, 29 Jan 2020 19:19:58 +0000 Subject: Re-eval snippet with emoji reaction If the eval message is edited after less than 10 seconds, an emoji is added to the message, and if the user adds the same, the snippet is re-evaluated. This make easier to correct snipper mistakes. --- bot/cogs/snekbox.py | 69 +++++++++++++++++++++++++++++++++++------------------ 1 file changed, 46 insertions(+), 23 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 81951efd3..1688c0278 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -1,3 +1,4 @@ +import asyncio import datetime import logging import re @@ -200,32 +201,54 @@ class Snekbox(Cog): log.info(f"Received code from {ctx.author} for evaluation:\n{code}") - self.jobs[ctx.author.id] = datetime.datetime.now() - code = self.prepare_input(code) + while True: + self.jobs[ctx.author.id] = datetime.datetime.now() + code = self.prepare_input(code) - try: - async with ctx.typing(): - results = await self.post_eval(code) - msg, error = self.get_results_message(results) - - if error: - output, paste_link = error, None - else: - output, paste_link = await self.format_output(results["stdout"]) - - icon = self.get_status_emoji(results) - msg = f"{ctx.author.mention} {icon} {msg}.\n\n```py\n{output}\n```" - if paste_link: - msg = f"{msg}\nFull output: {paste_link}" - - response = await ctx.send(msg) - self.bot.loop.create_task( - wait_for_deletion(response, user_ids=(ctx.author.id,), client=ctx.bot) + try: + async with ctx.typing(): + results = await self.post_eval(code) + msg, error = self.get_results_message(results) + + if error: + output, paste_link = error, None + else: + output, paste_link = await self.format_output(results["stdout"]) + + icon = self.get_status_emoji(results) + msg = f"{ctx.author.mention} {icon} {msg}.\n\n```py\n{output}\n```" + if paste_link: + msg = f"{msg}\nFull output: {paste_link}" + + response = await ctx.send(msg) + self.bot.loop.create_task( + wait_for_deletion(response, user_ids=(ctx.author.id,), client=ctx.bot) + ) + + log.info(f"{ctx.author}'s job had a return code of {results['returncode']}") + finally: + del self.jobs[ctx.author.id] + + try: + _, new_message = await self.bot.wait_for( + 'message_edit', + check=lambda o, n: n.id == ctx.message.id and o.content != n.content, + timeout=10 + ) + await ctx.message.add_reaction('🔁') + await self.bot.wait_for( + 'reaction_add', + check=lambda r, u: r.message.id == ctx.message.id and u.id == ctx.author.id and str(r) == '🔁', + timeout=10 ) - log.info(f"{ctx.author}'s job had a return code of {results['returncode']}") - finally: - del self.jobs[ctx.author.id] + log.info(f"Re-evaluating message {ctx.message.id}") + code = new_message.content.split(' ', maxsplit=1)[1] + await ctx.message.clear_reactions() + await response.delete() + except asyncio.TimeoutError: + await ctx.message.clear_reactions() + return def setup(bot: Bot) -> None: -- cgit v1.2.3 From 032e1b80934194b85c43d67f3a26cf51b972696d Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Sun, 9 Feb 2020 17:03:35 +0100 Subject: Use actual functions instead of lambdas for bot.wait_for The use of lambdas made the functions hard to test, this new format allows us to easily test those functions and document them. --- bot/cogs/snekbox.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 1688c0278..3fc8d9937 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -3,9 +3,11 @@ import datetime import logging import re import textwrap +from functools import partial from signal import Signals from typing import Optional, Tuple +from discord import Message, Reaction, User from discord.ext.commands import Cog, Context, command, guild_only from bot.bot import Bot @@ -232,13 +234,13 @@ class Snekbox(Cog): try: _, new_message = await self.bot.wait_for( 'message_edit', - check=lambda o, n: n.id == ctx.message.id and o.content != n.content, + check=partial(predicate_eval_message_edit, ctx), timeout=10 ) await ctx.message.add_reaction('🔁') await self.bot.wait_for( 'reaction_add', - check=lambda r, u: r.message.id == ctx.message.id and u.id == ctx.author.id and str(r) == '🔁', + check=partial(predicate_eval_emoji_reaction, ctx), timeout=10 ) @@ -251,6 +253,16 @@ class Snekbox(Cog): return +def predicate_eval_message_edit(ctx: Context, old_msg: Message, new_msg: Message) -> bool: + """Return True if the edited message is the context message and the content was indeed modified.""" + return new_msg.id == ctx.message.id and old_msg.content != new_msg.content + + +def predicate_eval_emoji_reaction(ctx: Context, reaction: Reaction, user: User) -> bool: + """Return True if the reaction 🔁 was added by the context message author on this message.""" + return reaction.message.id == ctx.message.id and user.id == ctx.author.id and str(reaction) == '🔁' + + def setup(bot: Bot) -> None: """Load the Snekbox cog.""" bot.add_cog(Snekbox(bot)) -- cgit v1.2.3 From adf63e65a0b861cb02a6e8cc1e5b2c2c09e57726 Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Sun, 9 Feb 2020 17:05:09 +0100 Subject: Create an AsyncContextManagerMock mock for testing asynchronous context managers It can be used to test aiohttp request functions, since they are async context managers --- tests/helpers.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/helpers.py b/tests/helpers.py index 5df796c23..6aee8623f 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -127,6 +127,18 @@ class AsyncMock(CustomMockMixin, unittest.mock.MagicMock): return super().__call__(*args, **kwargs) +class AsyncContextManagerMock(unittest.mock.MagicMock): + def __init__(self, return_value: Any): + super().__init__() + self._return_value = return_value + + async def __aenter__(self): + return self._return_value + + async def __aexit__(self, *args): + pass + + class AsyncIteratorMock: """ A class to mock asynchronous iterators. -- cgit v1.2.3 From 8c8f8dd50f376c3398c04e7bbe76b5028b69ff83 Mon Sep 17 00:00:00 2001 From: Akarys42 Date: Sun, 9 Feb 2020 17:11:47 +0100 Subject: Write tests for bot/cogs/test_snekbox.py --- tests/bot/cogs/test_snekbox.py | 363 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 363 insertions(+) create mode 100644 tests/bot/cogs/test_snekbox.py diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py new file mode 100644 index 000000000..293efed0f --- /dev/null +++ b/tests/bot/cogs/test_snekbox.py @@ -0,0 +1,363 @@ +import asyncio +import logging +import unittest +from unittest.mock import MagicMock, Mock, call, patch + +from bot.cogs import snekbox +from bot.cogs.snekbox import Snekbox +from bot.constants import URLs +from tests.helpers import ( + AsyncContextManagerMock, AsyncMock, MockBot, MockContext, MockMessage, MockReaction, MockUser, async_test +) + + +class SnekboxTests(unittest.TestCase): + def setUp(self): + """Add mocked bot and cog to the instance.""" + self.bot = MockBot() + + self.mocked_post = MagicMock() + self.mocked_post.json = AsyncMock() + self.bot.http_session.post = MagicMock(return_value=AsyncContextManagerMock(self.mocked_post)) + + self.cog = Snekbox(bot=self.bot) + + @async_test + async def test_post_eval(self): + """Post the eval code to the URLs.snekbox_eval_api endpoint.""" + await self.cog.post_eval("import random") + self.bot.http_session.post.assert_called_once_with( + URLs.snekbox_eval_api, + json={"input": "import random"}, + raise_for_status=True + ) + + @async_test + async def test_upload_output_reject_too_long(self): + """Reject output longer than MAX_PASTE_LEN.""" + self.assertEqual(await self.cog.upload_output("-" * (snekbox.MAX_PASTE_LEN + 1)), "too long to upload") + + @async_test + async def test_upload_output(self): + """Upload the eval output to the URLs.paste_service.format(key="documents") endpoint.""" + key = "RainbowDash" + self.mocked_post.json.return_value = {"key": key} + + self.assertEqual( + await self.cog.upload_output("My awesome output"), + URLs.paste_service.format(key=key) + ) + self.bot.http_session.post.assert_called_once_with( + URLs.paste_service.format(key="documents"), + data="My awesome output", + raise_for_status=True + ) + + @async_test + async def test_upload_output_gracefully_fallback_if_exception_during_request(self): + """Output upload gracefully fallback if the upload fail.""" + self.mocked_post.json.side_effect = Exception + log = logging.getLogger("bot.cogs.snekbox") + with self.assertLogs(logger=log, level='ERROR'): + await self.cog.upload_output('My awesome output!') + + @async_test + async def test_upload_output_gracefully_fallback_if_no_key_in_response(self): + """Output upload gracefully fallback if there is no key entry in the response body.""" + self.mocked_post.json.return_value = {} + self.assertEqual((await self.cog.upload_output('My awesome output!')), None) + + def test_prepare_input(self): + cases = ( + ('print("Hello world!")', 'print("Hello world!")', 'non-formatted'), + ('`print("Hello world!")`', 'print("Hello world!")', 'one line code block'), + ('```\nprint("Hello world!")```', 'print("Hello world!")', 'multiline code block'), + ('```py\nprint("Hello world!")```', 'print("Hello world!")', 'multiline python code block'), + ) + for case, expected, testname in cases: + with self.subTest(msg=f'Extract code from {testname}.', case=case, expected=expected): + self.assertEqual(self.cog.prepare_input(case), expected) + + def test_get_results_message(self): + """Return error and message according to the eval result.""" + cases = ( + ('ERROR', None, ('Your eval job has failed', 'ERROR')), + ('', 128 + snekbox.SIGKILL, ('Your eval job timed out or ran out of memory', '')), + ('', 255, ('Your eval job has failed', 'A fatal NsJail error occurred')) + ) + for stdout, returncode, expected in cases: + with self.subTest(stdout=stdout, returncode=returncode, expected=expected): + self.assertEqual(self.cog.get_results_message({'stdout': stdout, 'returncode': returncode}), expected) + + @patch('bot.cogs.snekbox.Signals', side_effect=ValueError) + def test_get_results_message_invalid_signal(self, mock_Signals: Mock): + self.assertEqual( + self.cog.get_results_message({'stdout': '', 'returncode': 127}), + ('Your eval job has completed with return code 127', '') + ) + + @patch('bot.cogs.snekbox.Signals') + def test_get_results_message_valid_signal(self, mock_Signals: Mock): + mock_Signals.return_value.name = 'SIGTEST' + self.assertEqual( + self.cog.get_results_message({'stdout': '', 'returncode': 127}), + ('Your eval job has completed with return code 127 (SIGTEST)', '') + ) + + def test_get_status_emoji(self): + """Return emoji according to the eval result.""" + cases = ( + ('', -1, ':warning:'), + ('Hello world!', 0, ':white_check_mark:'), + ('Invalid beard size', -1, ':x:') + ) + for stdout, returncode, expected in cases: + with self.subTest(stdout=stdout, returncode=returncode, expected=expected): + self.assertEqual(self.cog.get_status_emoji({'stdout': stdout, 'returncode': returncode}), expected) + + @async_test + async def test_format_output(self): + """Test output formatting.""" + self.cog.upload_output = AsyncMock(return_value='https://testificate.com/') + + too_many_lines = ( + '001 | v\n002 | e\n003 | r\n004 | y\n005 | l\n006 | o\n' + '007 | n\n008 | g\n009 | b\n010 | e\n011 | a\n... (truncated - too many lines)' + ) + too_long_too_many_lines = ( + "\n".join( + f"{i:03d} | {line}" for i, line in enumerate(['verylongbeard' * 10] * 15, 1) + )[:1000] + "\n... (truncated - too long, too many lines)" + ) + + cases = ( + ('', ('[No output]', None), 'No output'), + ('My awesome output', ('My awesome output', None), 'One line output'), + ('<@', ("<@\u200B", None), r'Convert <@ to <@\u200B'), + (' Date: Sat, 15 Feb 2020 19:57:58 -0800 Subject: Scheduler: fix #754 - only suppress CancelledError --- bot/utils/scheduling.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py index ee6c0a8e6..8d4721d70 100644 --- a/bot/utils/scheduling.py +++ b/bot/utils/scheduling.py @@ -63,12 +63,13 @@ def create_task(loop: asyncio.AbstractEventLoop, coro_or_future: Union[Coroutine """Creates an asyncio.Task object from a coroutine or future object.""" task: asyncio.Task = asyncio.ensure_future(coro_or_future, loop=loop) - # Silently ignore exceptions in a callback (handles the CancelledError nonsense) - task.add_done_callback(_silent_exception) + # Silently ignore CancelledError in a callback + task.add_done_callback(_suppress_cancelled_error) return task -def _silent_exception(future: asyncio.Future) -> None: - """Suppress future's exception.""" - with contextlib.suppress(Exception): - future.exception() +def _suppress_cancelled_error(future: asyncio.Future) -> None: + """Suppress future's CancelledError exception.""" + if future.cancelled(): + with contextlib.suppress(asyncio.CancelledError): + future.exception() -- cgit v1.2.3 From f905f73451730fb5b83b441f8d32748acef374e0 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 15 Feb 2020 20:07:26 -0800 Subject: Scheduler: remove create_task function It's redundant because the done callback only takes a single line to add and can be added in schedule_task(). * Use Task as the type hint rather than Future for _suppress_cancelled_error() --- bot/utils/scheduling.py | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py index 8d4721d70..7b055f5e7 100644 --- a/bot/utils/scheduling.py +++ b/bot/utils/scheduling.py @@ -2,7 +2,7 @@ import asyncio import contextlib import logging from abc import abstractmethod -from typing import Coroutine, Dict, Union +from typing import Dict from bot.utils import CogABCMeta @@ -41,7 +41,8 @@ class Scheduler(metaclass=CogABCMeta): ) return - task: asyncio.Task = create_task(loop, self._scheduled_task(task_data)) + task = loop.create_task(self._scheduled_task(task_data)) + task.add_done_callback(_suppress_cancelled_error) self.scheduled_tasks[task_id] = task log.debug(f"{self.cog_name}: scheduled task #{task_id}.") @@ -59,17 +60,8 @@ class Scheduler(metaclass=CogABCMeta): del self.scheduled_tasks[task_id] -def create_task(loop: asyncio.AbstractEventLoop, coro_or_future: Union[Coroutine, asyncio.Future]) -> asyncio.Task: - """Creates an asyncio.Task object from a coroutine or future object.""" - task: asyncio.Task = asyncio.ensure_future(coro_or_future, loop=loop) - - # Silently ignore CancelledError in a callback - task.add_done_callback(_suppress_cancelled_error) - return task - - -def _suppress_cancelled_error(future: asyncio.Future) -> None: - """Suppress future's CancelledError exception.""" - if future.cancelled(): +def _suppress_cancelled_error(task: asyncio.Task) -> None: + """Suppress a task's CancelledError exception.""" + if task.cancelled(): with contextlib.suppress(asyncio.CancelledError): - future.exception() + task.exception() -- cgit v1.2.3 From f5cd7e357de26c81b462b8935ec4bdaa032429fc Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 15 Feb 2020 20:10:42 -0800 Subject: Scheduler: correct schedule_task's docstring --- bot/utils/scheduling.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py index 7b055f5e7..adf10d683 100644 --- a/bot/utils/scheduling.py +++ b/bot/utils/scheduling.py @@ -33,7 +33,7 @@ class Scheduler(metaclass=CogABCMeta): """ Schedules a task. - `task_data` is passed to `Scheduler._scheduled_expiration` + `task_data` is passed to the `Scheduler._scheduled_task()` coroutine. """ if task_id in self.scheduled_tasks: log.debug( -- cgit v1.2.3 From 0d05be37564b1ec8babd688fb348c2c13eeb9fa2 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 15 Feb 2020 20:16:30 -0800 Subject: Scheduler: remove loop parameter from schedule_task asyncio.create_task() exists and will already use the running loop in the current thread. Because there is no intention of using a different loop in a different thread anywhere in the program for the foreseeable future, the loop parameter is redundant. --- bot/cogs/moderation/management.py | 4 +--- bot/cogs/moderation/scheduler.py | 4 ++-- bot/cogs/moderation/superstarify.py | 2 +- bot/cogs/reminders.py | 11 +++-------- bot/utils/scheduling.py | 4 ++-- 5 files changed, 9 insertions(+), 16 deletions(-) diff --git a/bot/cogs/moderation/management.py b/bot/cogs/moderation/management.py index f2964cd78..279c8b809 100644 --- a/bot/cogs/moderation/management.py +++ b/bot/cogs/moderation/management.py @@ -1,4 +1,3 @@ -import asyncio import logging import textwrap import typing as t @@ -133,8 +132,7 @@ class ModManagement(commands.Cog): # If the infraction was not marked as permanent, schedule a new expiration task if request_data['expires_at']: - loop = asyncio.get_event_loop() - self.infractions_cog.schedule_task(loop, new_infraction['id'], new_infraction) + self.infractions_cog.schedule_task(new_infraction['id'], new_infraction) log_text += f""" Previous expiry: {old_infraction['expires_at'] or "Permanent"} diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index e14c302cb..62b040d1f 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -48,7 +48,7 @@ class InfractionScheduler(Scheduler): ) for infraction in infractions: if infraction["expires_at"] is not None and infraction["type"] in supported_infractions: - self.schedule_task(self.bot.loop, infraction["id"], infraction) + self.schedule_task(infraction["id"], infraction) async def reapply_infraction( self, @@ -150,7 +150,7 @@ class InfractionScheduler(Scheduler): await action_coro if expiry: # Schedule the expiration of the infraction. - self.schedule_task(ctx.bot.loop, infraction["id"], infraction) + self.schedule_task(infraction["id"], infraction) except discord.HTTPException as e: # Accordingly display that applying the infraction failed. confirm_msg = f":x: failed to apply" diff --git a/bot/cogs/moderation/superstarify.py b/bot/cogs/moderation/superstarify.py index 050c847ac..d94ee6891 100644 --- a/bot/cogs/moderation/superstarify.py +++ b/bot/cogs/moderation/superstarify.py @@ -145,7 +145,7 @@ class Superstarify(InfractionScheduler, Cog): log.debug(f"Changing nickname of {member} to {forced_nick}.") self.mod_log.ignore(constants.Event.member_update, member.id) await member.edit(nick=forced_nick, reason=reason) - self.schedule_task(ctx.bot.loop, id_, infraction) + self.schedule_task(id_, infraction) # Send a DM to the user to notify them of their new infraction. await utils.notify_infraction( diff --git a/bot/cogs/reminders.py b/bot/cogs/reminders.py index 45bf9a8f4..d96dedd20 100644 --- a/bot/cogs/reminders.py +++ b/bot/cogs/reminders.py @@ -1,4 +1,3 @@ -import asyncio import logging import random import textwrap @@ -42,7 +41,6 @@ class Reminders(Scheduler, Cog): ) now = datetime.utcnow() - loop = asyncio.get_event_loop() for reminder in response: remind_at = datetime.fromisoformat(reminder['expiration'][:-1]) @@ -53,7 +51,7 @@ class Reminders(Scheduler, Cog): await self.send_reminder(reminder, late) else: - self.schedule_task(loop, reminder["id"], reminder) + self.schedule_task(reminder["id"], reminder) @staticmethod async def _send_confirmation(ctx: Context, on_success: str) -> None: @@ -88,10 +86,8 @@ class Reminders(Scheduler, Cog): async def _reschedule_reminder(self, reminder: dict) -> None: """Reschedule a reminder object.""" - loop = asyncio.get_event_loop() - self.cancel_task(reminder["id"]) - self.schedule_task(loop, reminder["id"], reminder) + self.schedule_task(reminder["id"], reminder) async def send_reminder(self, reminder: dict, late: relativedelta = None) -> None: """Send the reminder.""" @@ -185,8 +181,7 @@ class Reminders(Scheduler, Cog): on_success=f"Your reminder will arrive in {humanize_delta(relativedelta(expiration, now))}!" ) - loop = asyncio.get_event_loop() - self.schedule_task(loop, reminder["id"], reminder) + self.schedule_task(reminder["id"], reminder) @remind_group.command(name="list") async def list_reminders(self, ctx: Context) -> Optional[Message]: diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py index adf10d683..a16900066 100644 --- a/bot/utils/scheduling.py +++ b/bot/utils/scheduling.py @@ -29,7 +29,7 @@ class Scheduler(metaclass=CogABCMeta): then make a site API request to delete the reminder from the database. """ - def schedule_task(self, loop: asyncio.AbstractEventLoop, task_id: str, task_data: dict) -> None: + def schedule_task(self, task_id: str, task_data: dict) -> None: """ Schedules a task. @@ -41,7 +41,7 @@ class Scheduler(metaclass=CogABCMeta): ) return - task = loop.create_task(self._scheduled_task(task_data)) + task = asyncio.create_task(self._scheduled_task(task_data)) task.add_done_callback(_suppress_cancelled_error) self.scheduled_tasks[task_id] = task -- cgit v1.2.3 From 6b7c0a7a74460ee96c5ce574bf042f3de38dd685 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 15 Feb 2020 20:24:48 -0800 Subject: Scheduler: raise task exceptions besides CancelledError Explicitly retrieves the task's exception, which will raise the exception if one exists. * Rename _suppress_cancelled_error to _handle_task_exception --- bot/utils/scheduling.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py index a16900066..df46ccdd9 100644 --- a/bot/utils/scheduling.py +++ b/bot/utils/scheduling.py @@ -13,8 +13,9 @@ class Scheduler(metaclass=CogABCMeta): """Task scheduler.""" def __init__(self): + # Keep track of the child cog's name so the logs are clear. + self.cog_name = self.__class__.__name__ - self.cog_name = self.__class__.__name__ # keep track of the child cog's name so the logs are clear. self.scheduled_tasks: Dict[str, asyncio.Task] = {} @abstractmethod @@ -42,7 +43,7 @@ class Scheduler(metaclass=CogABCMeta): return task = asyncio.create_task(self._scheduled_task(task_data)) - task.add_done_callback(_suppress_cancelled_error) + task.add_done_callback(_handle_task_exception) self.scheduled_tasks[task_id] = task log.debug(f"{self.cog_name}: scheduled task #{task_id}.") @@ -60,8 +61,10 @@ class Scheduler(metaclass=CogABCMeta): del self.scheduled_tasks[task_id] -def _suppress_cancelled_error(task: asyncio.Task) -> None: - """Suppress a task's CancelledError exception.""" +def _handle_task_exception(task: asyncio.Task) -> None: + """Raise the task's exception, if any, unless the task is cancelled and has a CancelledError.""" if task.cancelled(): with contextlib.suppress(asyncio.CancelledError): task.exception() + else: + task.exception() -- cgit v1.2.3 From 687b6404e8976ffdc67ba9492a4355819f06a2f7 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 15 Feb 2020 21:17:22 -0800 Subject: Scheduler: cancel the task in the callback This design makes more sense and is more convenient than requiring tasks to be responsible for cancelling themselves. * Rename _handle_task_exception to _task_done_callback * Add trace logging --- bot/cogs/reminders.py | 6 +++--- bot/utils/scheduling.py | 36 +++++++++++++++++++++++++++--------- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/bot/cogs/reminders.py b/bot/cogs/reminders.py index d96dedd20..603b627fb 100644 --- a/bot/cogs/reminders.py +++ b/bot/cogs/reminders.py @@ -74,9 +74,6 @@ class Reminders(Scheduler, Cog): log.debug(f"Deleting reminder {reminder_id} (the user has been reminded).") await self._delete_reminder(reminder_id) - # Now we can begone with it from our schedule list. - self.cancel_task(reminder_id) - async def _delete_reminder(self, reminder_id: str) -> None: """Delete a reminder from the database, given its ID, and cancel the running task.""" await self.bot.api_client.delete('bot/reminders/' + str(reminder_id)) @@ -86,7 +83,10 @@ class Reminders(Scheduler, Cog): async def _reschedule_reminder(self, reminder: dict) -> None: """Reschedule a reminder object.""" + log.trace(f"Cancelling old task #{reminder['id']}") self.cancel_task(reminder["id"]) + + log.trace(f"Scheduling new task #{reminder['id']}") self.schedule_task(reminder["id"], reminder) async def send_reminder(self, reminder: dict, late: relativedelta = None) -> None: diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py index df46ccdd9..40d26249f 100644 --- a/bot/utils/scheduling.py +++ b/bot/utils/scheduling.py @@ -2,6 +2,7 @@ import asyncio import contextlib import logging from abc import abstractmethod +from functools import partial from typing import Dict from bot.utils import CogABCMeta @@ -36,6 +37,8 @@ class Scheduler(metaclass=CogABCMeta): `task_data` is passed to the `Scheduler._scheduled_task()` coroutine. """ + log.trace(f"{self.cog_name}: scheduling task #{task_id}...") + if task_id in self.scheduled_tasks: log.debug( f"{self.cog_name}: did not schedule task #{task_id}; task was already scheduled." @@ -43,13 +46,15 @@ class Scheduler(metaclass=CogABCMeta): return task = asyncio.create_task(self._scheduled_task(task_data)) - task.add_done_callback(_handle_task_exception) + task.add_done_callback(partial(self._task_done_callback, task_id)) self.scheduled_tasks[task_id] = task - log.debug(f"{self.cog_name}: scheduled task #{task_id}.") + log.debug(f"{self.cog_name}: scheduled task #{task_id} {id(task)}.") def cancel_task(self, task_id: str) -> None: """Un-schedules a task.""" + log.trace(f"{self.cog_name}: cancelling task #{task_id}...") + task = self.scheduled_tasks.get(task_id) if task is None: @@ -57,14 +62,27 @@ class Scheduler(metaclass=CogABCMeta): return task.cancel() - log.debug(f"{self.cog_name}: unscheduled task #{task_id}.") + log.debug(f"{self.cog_name}: unscheduled task #{task_id} {id(task)}.") del self.scheduled_tasks[task_id] + def _task_done_callback(self, task_id: str, task: asyncio.Task) -> None: + """ + Unschedule the task and raise its exception if one exists. + + If the task was cancelled, the CancelledError is retrieved and suppressed. In this case, + the task is already assumed to have been unscheduled. + """ + log.trace(f"{self.cog_name}: performing done callback for task #{task_id} {id(task)}") + + if task.cancelled(): + with contextlib.suppress(asyncio.CancelledError): + task.exception() + else: + # Check if it exists to avoid logging a warning. + if task_id in self.scheduled_tasks: + # Only cancel if the task is not cancelled to avoid a race condition when a new + # task is scheduled using the same ID. Reminders do this when re-scheduling after + # editing. + self.cancel_task(task_id) -def _handle_task_exception(task: asyncio.Task) -> None: - """Raise the task's exception, if any, unless the task is cancelled and has a CancelledError.""" - if task.cancelled(): - with contextlib.suppress(asyncio.CancelledError): task.exception() - else: - task.exception() -- cgit v1.2.3 From 6edae6cda82add4d0bf538e916ce571432f83ab1 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 15 Feb 2020 21:21:51 -0800 Subject: Moderation: avoid prematurely cancelling deactivation task Because deactivate_infraction() explicitly cancels the scheduled task, it now runs in a separate task to avoid prematurely cancelling itself. --- bot/cogs/moderation/scheduler.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 62b040d1f..7d401c7ff 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -415,4 +415,6 @@ class InfractionScheduler(Scheduler): expiry = dateutil.parser.isoparse(infraction["expires_at"]).replace(tzinfo=None) await time.wait_until(expiry) - await self.deactivate_infraction(infraction) + # Because deactivate_infraction() explicitly cancels this scheduled task, it runs in + # a separate task to avoid prematurely cancelling itself. + self.bot.loop.create_task(self.deactivate_infraction(infraction)) -- cgit v1.2.3 From f09d6b0646edb62806b60b145986b7cc680fd77c Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sat, 15 Feb 2020 21:34:38 -0800 Subject: Scheduler: make _scheduled_tasks private Main concern is someone trying to cancel a task directly. The workaround for the race condition relies on the task only being cancelled via Scheduler.cancel_task(), particularly because it removes the task from the dictionary. The done callback will not remove from the dictionary if it sees the task has already been cancelled. So it's a bad idea to cancel tasks directly... --- bot/utils/scheduling.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py index 40d26249f..0d66952eb 100644 --- a/bot/utils/scheduling.py +++ b/bot/utils/scheduling.py @@ -17,7 +17,7 @@ class Scheduler(metaclass=CogABCMeta): # Keep track of the child cog's name so the logs are clear. self.cog_name = self.__class__.__name__ - self.scheduled_tasks: Dict[str, asyncio.Task] = {} + self._scheduled_tasks: Dict[str, asyncio.Task] = {} @abstractmethod async def _scheduled_task(self, task_object: dict) -> None: @@ -39,7 +39,7 @@ class Scheduler(metaclass=CogABCMeta): """ log.trace(f"{self.cog_name}: scheduling task #{task_id}...") - if task_id in self.scheduled_tasks: + if task_id in self._scheduled_tasks: log.debug( f"{self.cog_name}: did not schedule task #{task_id}; task was already scheduled." ) @@ -48,14 +48,14 @@ class Scheduler(metaclass=CogABCMeta): task = asyncio.create_task(self._scheduled_task(task_data)) task.add_done_callback(partial(self._task_done_callback, task_id)) - self.scheduled_tasks[task_id] = task + self._scheduled_tasks[task_id] = task log.debug(f"{self.cog_name}: scheduled task #{task_id} {id(task)}.") def cancel_task(self, task_id: str) -> None: """Un-schedules a task.""" log.trace(f"{self.cog_name}: cancelling task #{task_id}...") - task = self.scheduled_tasks.get(task_id) + task = self._scheduled_tasks.get(task_id) if task is None: log.warning(f"{self.cog_name}: Failed to unschedule {task_id} (no task found).") @@ -63,7 +63,7 @@ class Scheduler(metaclass=CogABCMeta): task.cancel() log.debug(f"{self.cog_name}: unscheduled task #{task_id} {id(task)}.") - del self.scheduled_tasks[task_id] + del self._scheduled_tasks[task_id] def _task_done_callback(self, task_id: str, task: asyncio.Task) -> None: """ @@ -79,7 +79,7 @@ class Scheduler(metaclass=CogABCMeta): task.exception() else: # Check if it exists to avoid logging a warning. - if task_id in self.scheduled_tasks: + if task_id in self._scheduled_tasks: # Only cancel if the task is not cancelled to avoid a race condition when a new # task is scheduled using the same ID. Reminders do this when re-scheduling after # editing. -- cgit v1.2.3 From 6f25814aee1794fdabce6fef97d0d776121d5535 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 11:41:56 -0800 Subject: Moderation: fix member not found error not being shown --- bot/cogs/moderation/infractions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/moderation/infractions.py b/bot/cogs/moderation/infractions.py index f4e296df9..9ea17b2b3 100644 --- a/bot/cogs/moderation/infractions.py +++ b/bot/cogs/moderation/infractions.py @@ -313,6 +313,6 @@ class Infractions(InfractionScheduler, commands.Cog): async def cog_command_error(self, ctx: Context, error: Exception) -> None: """Send a notification to the invoking context on a Union failure.""" if isinstance(error, commands.BadUnionArgument): - if discord.User in error.converters: + if discord.User in error.converters or discord.Member in error.converters: await ctx.send(str(error.errors[0])) error.handled = True -- cgit v1.2.3 From a83d2683f72a750b1946df913749a5c4257ebb16 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 11:52:54 -0800 Subject: Error handler: create separate function to handle CheckFailure --- bot/cogs/error_handler.py | 44 +++++++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 52893b2ee..bd47eecf8 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -100,21 +100,9 @@ class ErrorHandler(Cog): f"Command {command} invoked by {ctx.message.author} with error " f"{e.__class__.__name__}: {e}" ) - elif isinstance(e, NoPrivateMessage): - await ctx.send("Sorry, this command can't be used in a private message!") - elif isinstance(e, BotMissingPermissions): - await ctx.send(f"Sorry, it looks like I don't have the permissions I need to do that.") - log.warning( - f"The bot is missing permissions to execute command {command}: {e.missing_perms}" - ) - elif isinstance(e, MissingPermissions): - log.debug( - f"{ctx.message.author} is missing permissions to invoke command {command}: " - f"{e.missing_perms}" - ) - elif isinstance(e, InChannelCheckFailure): - await ctx.send(e) - elif isinstance(e, (CheckFailure, CommandOnCooldown, DisabledCommand)): + elif isinstance(e, CheckFailure): + await self.handle_check_failure(ctx, e) + elif isinstance(e, (CommandOnCooldown, DisabledCommand)): log.debug( f"Command {command} invoked by {ctx.message.author} with error " f"{e.__class__.__name__}: {e}" @@ -138,8 +126,34 @@ class ErrorHandler(Cog): else: await self.handle_unexpected_error(ctx, e.original) else: + # MaxConcurrencyReached, ExtensionError await self.handle_unexpected_error(ctx, e) + @staticmethod + async def handle_check_failure(ctx: Context, e: CheckFailure) -> None: + """Handle CheckFailure exceptions and its children.""" + command = ctx.command + + if isinstance(e, NoPrivateMessage): + await ctx.send("Sorry, this command can't be used in a private message!") + elif isinstance(e, BotMissingPermissions): + await ctx.send(f"Sorry, it looks like I don't have the permissions I need to do that.") + log.warning( + f"The bot is missing permissions to execute command {command}: {e.missing_perms}" + ) + elif isinstance(e, MissingPermissions): + log.debug( + f"{ctx.message.author} is missing permissions to invoke command {command}: " + f"{e.missing_perms}" + ) + elif isinstance(e, InChannelCheckFailure): + await ctx.send(e) + else: + log.debug( + f"Command {command} invoked by {ctx.message.author} with error " + f"{e.__class__.__name__}: {e}" + ) + @staticmethod async def handle_unexpected_error(ctx: Context, e: CommandError) -> None: """Generic handler for errors without an explicit handler.""" -- cgit v1.2.3 From eab4b16ccb2d5b6f3d0a8765e8741fe88fb03e27 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 11:58:48 -0800 Subject: Error handler: create separate function to handle ResponseCodeError --- bot/cogs/error_handler.py | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index bd47eecf8..97124cb15 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -109,20 +109,7 @@ class ErrorHandler(Cog): ) elif isinstance(e, CommandInvokeError): if isinstance(e.original, ResponseCodeError): - status = e.original.response.status - - if status == 404: - await ctx.send("There does not seem to be anything matching your query.") - elif status == 400: - content = await e.original.response.json() - log.debug(f"API responded with 400 for command {command}: %r.", content) - await ctx.send("According to the API, your request is malformed.") - elif 500 <= status < 600: - await ctx.send("Sorry, there seems to be an internal issue with the API.") - log.warning(f"API responded with {status} for command {command}") - else: - await ctx.send(f"Got an unexpected status code from the API (`{status}`).") - log.warning(f"Unexpected API response for command {command}: {status}") + await self.handle_api_error(ctx, e.original) else: await self.handle_unexpected_error(ctx, e.original) else: @@ -154,6 +141,22 @@ class ErrorHandler(Cog): f"{e.__class__.__name__}: {e}" ) + @staticmethod + async def handle_api_error(ctx: Context, e: ResponseCodeError) -> None: + """Handle ResponseCodeError exceptions.""" + if e.status == 404: + await ctx.send("There does not seem to be anything matching your query.") + elif e.status == 400: + content = await e.response.json() + log.debug(f"API responded with 400 for command {ctx.command}: %r.", content) + await ctx.send("According to the API, your request is malformed.") + elif 500 <= e.status < 600: + await ctx.send("Sorry, there seems to be an internal issue with the API.") + log.warning(f"API responded with {e.status} for command {ctx.command}") + else: + await ctx.send(f"Got an unexpected status code from the API (`{e.status}`).") + log.warning(f"Unexpected API response for command {ctx.command}: {e.status}") + @staticmethod async def handle_unexpected_error(ctx: Context, e: CommandError) -> None: """Generic handler for errors without an explicit handler.""" -- cgit v1.2.3 From 29e3c3e46242866820b9c4461378ed4b2e3afb47 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 12:09:24 -0800 Subject: Error handler: log unhandled exceptions instead of re-raising --- bot/cogs/error_handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 97124cb15..5eef045e8 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -165,9 +165,9 @@ class ErrorHandler(Cog): f"```{e.__class__.__name__}: {e}```" ) log.error( - f"Error executing command invoked by {ctx.message.author}: {ctx.message.content}" + f"Error executing command invoked by {ctx.message.author}: {ctx.message.content}", + exc_info=e ) - raise e def setup(bot: Bot) -> None: -- cgit v1.2.3 From 806c69f78c5751f6dc93bd8dcc6fff95436fe0ed Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 12:19:52 -0800 Subject: Error handler: move tag retrieval to a separate function --- bot/cogs/error_handler.py | 46 ++++++++++++++++++++++++++++------------------ 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 5eef045e8..7078d425d 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -72,24 +72,8 @@ class ErrorHandler(Cog): # Try to look for a tag with the command's name if the command isn't found. if isinstance(e, CommandNotFound) and not hasattr(ctx, "invoked_from_error_handler"): - if not ctx.channel.id == Channels.verification: - tags_get_command = self.bot.get_command("tags get") - ctx.invoked_from_error_handler = True - - log_msg = "Cancelling attempt to fall back to a tag due to failed checks." - try: - if not await tags_get_command.can_run(ctx): - log.debug(log_msg) - return - except CommandError as tag_error: - log.debug(log_msg) - await self.on_command_error(ctx, tag_error) - return - - # Return to not raise the exception - with contextlib.suppress(ResponseCodeError): - await ctx.invoke(tags_get_command, tag_name=ctx.invoked_with) - return + if ctx.channel.id != Channels.verification: + await self.try_get_tag(ctx) elif isinstance(e, BadArgument): await ctx.send(f"Bad argument: {e}\n") await ctx.invoke(*help_command) @@ -116,6 +100,32 @@ class ErrorHandler(Cog): # MaxConcurrencyReached, ExtensionError await self.handle_unexpected_error(ctx, e) + async def try_get_tag(self, ctx: Context) -> None: + """ + Attempt to display a tag by interpreting the command name as a tag name. + + The invocation of tags get respects its checks. Any CommandErrors raised will be handled + by `on_command_error`, but the `invoked_from_error_handler` attribute will be added to + the context to prevent infinite recursion in the case of a CommandNotFound exception. + """ + tags_get_command = self.bot.get_command("tags get") + ctx.invoked_from_error_handler = True + + log_msg = "Cancelling attempt to fall back to a tag due to failed checks." + try: + if not await tags_get_command.can_run(ctx): + log.debug(log_msg) + return + except CommandError as tag_error: + log.debug(log_msg) + await self.on_command_error(ctx, tag_error) + return + + # Return to not raise the exception + with contextlib.suppress(ResponseCodeError): + await ctx.invoke(tags_get_command, tag_name=ctx.invoked_with) + return + @staticmethod async def handle_check_failure(ctx: Context, e: CheckFailure) -> None: """Handle CheckFailure exceptions and its children.""" -- cgit v1.2.3 From fb30fb1427fa26d6cfd54fdb6a80e4e7552d808f Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 12:31:42 -0800 Subject: Error handler: move help command retrieval to a separate function --- bot/cogs/error_handler.py | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 7078d425d..6a0aef13e 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -1,10 +1,12 @@ import contextlib import logging +import typing as t from discord.ext.commands import ( BadArgument, BotMissingPermissions, CheckFailure, + Command, CommandError, CommandInvokeError, CommandNotFound, @@ -53,18 +55,9 @@ class ErrorHandler(Cog): 10. Otherwise, handling is deferred to `handle_unexpected_error` """ command = ctx.command - parent = None - if command is not None: - parent = command.parent - - # Retrieve the help command for the invoked command. - if parent and command: - help_command = (self.bot.get_command("help"), parent.name, command.name) - elif command: - help_command = (self.bot.get_command("help"), command.name) - else: - help_command = (self.bot.get_command("help"),) + # TODO: use ctx.send_help() once PR #519 is merged. + help_command = await self.get_help_command(command) if hasattr(e, "handled"): log.trace(f"Command {command} had its error already handled locally; ignoring.") @@ -100,6 +93,20 @@ class ErrorHandler(Cog): # MaxConcurrencyReached, ExtensionError await self.handle_unexpected_error(ctx, e) + async def get_help_command(self, command: t.Optional[Command]) -> t.Tuple: + """Return the help command invocation args to display help for `command`.""" + parent = None + if command is not None: + parent = command.parent + + # Retrieve the help command for the invoked command. + if parent and command: + return self.bot.get_command("help"), parent.name, command.name + elif command: + return self.bot.get_command("help"), command.name + else: + return self.bot.get_command("help") + async def try_get_tag(self, ctx: Context) -> None: """ Attempt to display a tag by interpreting the command name as a tag name. -- cgit v1.2.3 From d263f948e57a71e23cf4e04d678a880a130f3884 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 12:44:58 -0800 Subject: Error handler: create separate function to handle UserInputError --- bot/cogs/error_handler.py | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 6a0aef13e..c7758d946 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -56,9 +56,6 @@ class ErrorHandler(Cog): """ command = ctx.command - # TODO: use ctx.send_help() once PR #519 is merged. - help_command = await self.get_help_command(command) - if hasattr(e, "handled"): log.trace(f"Command {command} had its error already handled locally; ignoring.") return @@ -67,16 +64,8 @@ class ErrorHandler(Cog): if isinstance(e, CommandNotFound) and not hasattr(ctx, "invoked_from_error_handler"): if ctx.channel.id != Channels.verification: await self.try_get_tag(ctx) - elif isinstance(e, BadArgument): - await ctx.send(f"Bad argument: {e}\n") - await ctx.invoke(*help_command) elif isinstance(e, UserInputError): - await ctx.send("Something about your input seems off. Check the arguments:") - await ctx.invoke(*help_command) - log.debug( - f"Command {command} invoked by {ctx.message.author} with error " - f"{e.__class__.__name__}: {e}" - ) + await self.handle_user_input_error(ctx, e) elif isinstance(e, CheckFailure): await self.handle_check_failure(ctx, e) elif isinstance(e, (CommandOnCooldown, DisabledCommand)): @@ -133,6 +122,22 @@ class ErrorHandler(Cog): await ctx.invoke(tags_get_command, tag_name=ctx.invoked_with) return + async def handle_user_input_error(self, ctx: Context, e: UserInputError) -> None: + """Handle UserInputError exceptions and its children.""" + # TODO: use ctx.send_help() once PR #519 is merged. + help_command = await self.get_help_command(ctx.command) + + if isinstance(e, BadArgument): + await ctx.send(f"Bad argument: {e}\n") + await ctx.invoke(*help_command) + else: + await ctx.send("Something about your input seems off. Check the arguments:") + await ctx.invoke(*help_command) + log.debug( + f"Command {ctx.command} invoked by {ctx.message.author} with error " + f"{e.__class__.__name__}: {e}" + ) + @staticmethod async def handle_check_failure(ctx: Context, e: CheckFailure) -> None: """Handle CheckFailure exceptions and its children.""" -- cgit v1.2.3 From dbd879e715fe9eadee33d098282cb7b4d941df26 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 13:09:05 -0800 Subject: Error handler: simplify error imports Import the errors module and qualify the error types with it rather than importing a large list of error types. --- bot/cogs/error_handler.py | 44 +++++++++++++++----------------------------- 1 file changed, 15 insertions(+), 29 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index c7758d946..c65ada344 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -2,21 +2,7 @@ import contextlib import logging import typing as t -from discord.ext.commands import ( - BadArgument, - BotMissingPermissions, - CheckFailure, - Command, - CommandError, - CommandInvokeError, - CommandNotFound, - CommandOnCooldown, - DisabledCommand, - MissingPermissions, - NoPrivateMessage, - UserInputError, -) -from discord.ext.commands import Cog, Context +from discord.ext.commands import Cog, Command, Context, errors from bot.api import ResponseCodeError from bot.bot import Bot @@ -33,7 +19,7 @@ class ErrorHandler(Cog): self.bot = bot @Cog.listener() - async def on_command_error(self, ctx: Context, e: CommandError) -> None: + async def on_command_error(self, ctx: Context, e: errors.CommandError) -> None: """ Provide generic command error handling. @@ -61,19 +47,19 @@ class ErrorHandler(Cog): return # Try to look for a tag with the command's name if the command isn't found. - if isinstance(e, CommandNotFound) and not hasattr(ctx, "invoked_from_error_handler"): + if isinstance(e, errors.CommandNotFound) and not hasattr(ctx, "invoked_from_error_handler"): if ctx.channel.id != Channels.verification: await self.try_get_tag(ctx) - elif isinstance(e, UserInputError): + elif isinstance(e, errors.UserInputError): await self.handle_user_input_error(ctx, e) - elif isinstance(e, CheckFailure): + elif isinstance(e, errors.CheckFailure): await self.handle_check_failure(ctx, e) - elif isinstance(e, (CommandOnCooldown, DisabledCommand)): + elif isinstance(e, (errors.CommandOnCooldown, errors.DisabledCommand)): log.debug( f"Command {command} invoked by {ctx.message.author} with error " f"{e.__class__.__name__}: {e}" ) - elif isinstance(e, CommandInvokeError): + elif isinstance(e, errors.CommandInvokeError): if isinstance(e.original, ResponseCodeError): await self.handle_api_error(ctx, e.original) else: @@ -112,7 +98,7 @@ class ErrorHandler(Cog): if not await tags_get_command.can_run(ctx): log.debug(log_msg) return - except CommandError as tag_error: + except errors.CommandError as tag_error: log.debug(log_msg) await self.on_command_error(ctx, tag_error) return @@ -122,12 +108,12 @@ class ErrorHandler(Cog): await ctx.invoke(tags_get_command, tag_name=ctx.invoked_with) return - async def handle_user_input_error(self, ctx: Context, e: UserInputError) -> None: + async def handle_user_input_error(self, ctx: Context, e: errors.UserInputError) -> None: """Handle UserInputError exceptions and its children.""" # TODO: use ctx.send_help() once PR #519 is merged. help_command = await self.get_help_command(ctx.command) - if isinstance(e, BadArgument): + if isinstance(e, errors.BadArgument): await ctx.send(f"Bad argument: {e}\n") await ctx.invoke(*help_command) else: @@ -139,18 +125,18 @@ class ErrorHandler(Cog): ) @staticmethod - async def handle_check_failure(ctx: Context, e: CheckFailure) -> None: + async def handle_check_failure(ctx: Context, e: errors.CheckFailure) -> None: """Handle CheckFailure exceptions and its children.""" command = ctx.command - if isinstance(e, NoPrivateMessage): + if isinstance(e, errors.NoPrivateMessage): await ctx.send("Sorry, this command can't be used in a private message!") - elif isinstance(e, BotMissingPermissions): + elif isinstance(e, errors.BotMissingPermissions): await ctx.send(f"Sorry, it looks like I don't have the permissions I need to do that.") log.warning( f"The bot is missing permissions to execute command {command}: {e.missing_perms}" ) - elif isinstance(e, MissingPermissions): + elif isinstance(e, errors.MissingPermissions): log.debug( f"{ctx.message.author} is missing permissions to invoke command {command}: " f"{e.missing_perms}" @@ -180,7 +166,7 @@ class ErrorHandler(Cog): log.warning(f"Unexpected API response for command {ctx.command}: {e.status}") @staticmethod - async def handle_unexpected_error(ctx: Context, e: CommandError) -> None: + async def handle_unexpected_error(ctx: Context, e: errors.CommandError) -> None: """Generic handler for errors without an explicit handler.""" await ctx.send( f"Sorry, an unexpected error occurred. Please let us know!\n\n" -- cgit v1.2.3 From d2f94f4c1280716e32e13611f6c778f9d9d4efd3 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 13:15:55 -0800 Subject: Error handler: handle MissingRequiredArgument Send a message indicating which argument is missing. --- bot/cogs/error_handler.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index c65ada344..ffb36d10a 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -113,7 +113,10 @@ class ErrorHandler(Cog): # TODO: use ctx.send_help() once PR #519 is merged. help_command = await self.get_help_command(ctx.command) - if isinstance(e, errors.BadArgument): + if isinstance(e, errors.MissingRequiredArgument): + await ctx.send(f"Missing required argument `{e.param.name}`.") + await ctx.invoke(*help_command) + elif isinstance(e, errors.BadArgument): await ctx.send(f"Bad argument: {e}\n") await ctx.invoke(*help_command) else: -- cgit v1.2.3 From 6fa0ba18a6b4daa265e6716f9d360117378c67ab Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 13:17:03 -0800 Subject: Error handler: handle TooManyArguments Send a message specifying the error reason. --- bot/cogs/error_handler.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index ffb36d10a..5cf95e71a 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -116,6 +116,9 @@ class ErrorHandler(Cog): if isinstance(e, errors.MissingRequiredArgument): await ctx.send(f"Missing required argument `{e.param.name}`.") await ctx.invoke(*help_command) + elif isinstance(e, errors.TooManyArguments): + await ctx.send(f"Too many arguments provided.") + await ctx.invoke(*help_command) elif isinstance(e, errors.BadArgument): await ctx.send(f"Bad argument: {e}\n") await ctx.invoke(*help_command) -- cgit v1.2.3 From 8cdbec386d50e5866dcfd7cc0aeee359bb182317 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 13:23:56 -0800 Subject: Error handler: handle BadUnionArgument Send a message specifying the parameter name, the converters used, and the last error message from the converters. --- bot/cogs/error_handler.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 5cf95e71a..d67261fc6 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -122,6 +122,8 @@ class ErrorHandler(Cog): elif isinstance(e, errors.BadArgument): await ctx.send(f"Bad argument: {e}\n") await ctx.invoke(*help_command) + elif isinstance(e, errors.BadUnionArgument): + await ctx.send(f"Bad argument: {e}\n```{e.errors[-1]}```") else: await ctx.send("Something about your input seems off. Check the arguments:") await ctx.invoke(*help_command) -- cgit v1.2.3 From 4116aca0218138dd1a97db39b942a886945fa05b Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 17:23:43 -0800 Subject: Error handler: handle ArgumentParsingError Simply send the error message with the help command. --- bot/cogs/error_handler.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index d67261fc6..07b93283d 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -124,6 +124,8 @@ class ErrorHandler(Cog): await ctx.invoke(*help_command) elif isinstance(e, errors.BadUnionArgument): await ctx.send(f"Bad argument: {e}\n```{e.errors[-1]}```") + elif isinstance(e, errors.ArgumentParsingError): + await ctx.send(f"Argument parsing error: {e}") else: await ctx.send("Something about your input seems off. Check the arguments:") await ctx.invoke(*help_command) -- cgit v1.2.3 From 476d5e7851f5b53ece319f023eeca88ae5c345eb Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 17:38:41 -0800 Subject: Error handler: (almost) always log the error being handled The log level is debug for most errors and it's mainly useful for precisely that - debugging. This is why some "useless" errors are also logged e.g. CommandNotFound. Unexpected errors and some API errors will still have higher levels. * Add a single log statement to the end of the handler to cover UserInputError, CheckFailure, and CommandNotFound (when it's not trying to get a tag) * Log 404s from API --- bot/cogs/error_handler.py | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 07b93283d..ff8b36ddc 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -50,23 +50,26 @@ class ErrorHandler(Cog): if isinstance(e, errors.CommandNotFound) and not hasattr(ctx, "invoked_from_error_handler"): if ctx.channel.id != Channels.verification: await self.try_get_tag(ctx) + return # Exit early to avoid logging. elif isinstance(e, errors.UserInputError): await self.handle_user_input_error(ctx, e) elif isinstance(e, errors.CheckFailure): await self.handle_check_failure(ctx, e) - elif isinstance(e, (errors.CommandOnCooldown, errors.DisabledCommand)): - log.debug( - f"Command {command} invoked by {ctx.message.author} with error " - f"{e.__class__.__name__}: {e}" - ) elif isinstance(e, errors.CommandInvokeError): if isinstance(e.original, ResponseCodeError): await self.handle_api_error(ctx, e.original) else: await self.handle_unexpected_error(ctx, e.original) - else: - # MaxConcurrencyReached, ExtensionError + return # Exit early to avoid logging. + elif not isinstance(e, (errors.CommandOnCooldown, errors.DisabledCommand)): + # ConversionError, MaxConcurrencyReached, ExtensionError await self.handle_unexpected_error(ctx, e) + return # Exit early to avoid logging. + + log.debug( + f"Command {command} invoked by {ctx.message.author} with error " + f"{e.__class__.__name__}: {e}" + ) async def get_help_command(self, command: t.Optional[Command]) -> t.Tuple: """Return the help command invocation args to display help for `command`.""" @@ -129,10 +132,6 @@ class ErrorHandler(Cog): else: await ctx.send("Something about your input seems off. Check the arguments:") await ctx.invoke(*help_command) - log.debug( - f"Command {ctx.command} invoked by {ctx.message.author} with error " - f"{e.__class__.__name__}: {e}" - ) @staticmethod async def handle_check_failure(ctx: Context, e: errors.CheckFailure) -> None: @@ -153,17 +152,13 @@ class ErrorHandler(Cog): ) elif isinstance(e, InChannelCheckFailure): await ctx.send(e) - else: - log.debug( - f"Command {command} invoked by {ctx.message.author} with error " - f"{e.__class__.__name__}: {e}" - ) @staticmethod async def handle_api_error(ctx: Context, e: ResponseCodeError) -> None: """Handle ResponseCodeError exceptions.""" if e.status == 404: await ctx.send("There does not seem to be anything matching your query.") + log.debug(f"API responded with 404 for command {ctx.command}") elif e.status == 400: content = await e.response.json() log.debug(f"API responded with 400 for command {ctx.command}: %r.", content) -- cgit v1.2.3 From 9bfdf7e3e95c07a1b0369c9fa8bdb4c91339732f Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 18:09:08 -0800 Subject: Error handler: simplify check failure handler & handle bot missing roles discord.py's default error messages are quite descriptive already so there really isn't a need to write our own. Therefore, the log calls were removed so that the generic debug log message is used in the on_command_error. In addition to handling missing bot permissions, missing bot roles are also handled. The message doesn't specify which because it doesn't really matter to the end-user. The logs will use the default error messages as described above, and those will contain the specific roles or permissions that are missing. --- bot/cogs/error_handler.py | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index ff8b36ddc..6c4074e3a 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -136,21 +136,17 @@ class ErrorHandler(Cog): @staticmethod async def handle_check_failure(ctx: Context, e: errors.CheckFailure) -> None: """Handle CheckFailure exceptions and its children.""" - command = ctx.command + bot_missing_errors = ( + errors.BotMissingPermissions, + errors.BotMissingRole, + errors.BotMissingAnyRole + ) - if isinstance(e, errors.NoPrivateMessage): - await ctx.send("Sorry, this command can't be used in a private message!") - elif isinstance(e, errors.BotMissingPermissions): - await ctx.send(f"Sorry, it looks like I don't have the permissions I need to do that.") - log.warning( - f"The bot is missing permissions to execute command {command}: {e.missing_perms}" - ) - elif isinstance(e, errors.MissingPermissions): - log.debug( - f"{ctx.message.author} is missing permissions to invoke command {command}: " - f"{e.missing_perms}" + if isinstance(e, bot_missing_errors): + await ctx.send( + f"Sorry, it looks like I don't have the permissions or roles I need to do that." ) - elif isinstance(e, InChannelCheckFailure): + elif isinstance(e, (InChannelCheckFailure, errors.NoPrivateMessage)): await ctx.send(e) @staticmethod -- cgit v1.2.3 From aa84854f942d68f5245d2ca99612dfdd6ad167ce Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 19:01:42 -0800 Subject: Error handler: update docstrings to reflect recent changes --- bot/cogs/error_handler.py | 59 +++++++++++++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 20 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 6c4074e3a..d2c806566 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -23,22 +23,22 @@ class ErrorHandler(Cog): """ Provide generic command error handling. - Error handling is deferred to any local error handler, if present. - - Error handling emits a single error response, prioritized as follows: - 1. If the name fails to match a command but matches a tag, the tag is invoked - 2. Send a BadArgument error message to the invoking context & invoke the command's help - 3. Send a UserInputError error message to the invoking context & invoke the command's help - 4. Send a NoPrivateMessage error message to the invoking context - 5. Send a BotMissingPermissions error message to the invoking context - 6. Log a MissingPermissions error, no message is sent - 7. Send a InChannelCheckFailure error message to the invoking context - 8. Log CheckFailure, CommandOnCooldown, and DisabledCommand errors, no message is sent - 9. For CommandInvokeErrors, response is based on the type of error: - * 404: Error message is sent to the invoking context - * 400: Log the resopnse JSON, no message is sent - * 500 <= status <= 600: Error message is sent to the invoking context - 10. Otherwise, handling is deferred to `handle_unexpected_error` + Error handling is deferred to any local error handler, if present. This is done by + checking for the presence of a `handled` attribute on the error. + + Error handling emits a single error message in the invoking context `ctx` and a log message, + prioritised as follows: + + 1. If the name fails to match a command but matches a tag, the tag is invoked + * If CommandNotFound is raised when invoking the tag (determined by the presence of the + `invoked_from_error_handler` attribute), this error is treated as being unexpected + and therefore sends an error message + * Commands in the verification channel are ignored + 2. UserInputError: see `handle_user_input_error` + 3. CheckFailure: see `handle_check_failure` + 4. ResponseCodeError: see `handle_api_error` + 5. Otherwise, if not a CommandOnCooldown or DisabledCommand, handling is deferred to + `handle_unexpected_error` """ command = ctx.command @@ -112,7 +112,16 @@ class ErrorHandler(Cog): return async def handle_user_input_error(self, ctx: Context, e: errors.UserInputError) -> None: - """Handle UserInputError exceptions and its children.""" + """ + Send an error message in `ctx` for UserInputError, sometimes invoking the help command too. + + * MissingRequiredArgument: send an error message with arg name and the help command + * TooManyArguments: send an error message and the help command + * BadArgument: send an error message and the help command + * BadUnionArgument: send an error message including the error produced by the last converter + * ArgumentParsingError: send an error message + * Other: send an error message and the help command + """ # TODO: use ctx.send_help() once PR #519 is merged. help_command = await self.get_help_command(ctx.command) @@ -135,7 +144,17 @@ class ErrorHandler(Cog): @staticmethod async def handle_check_failure(ctx: Context, e: errors.CheckFailure) -> None: - """Handle CheckFailure exceptions and its children.""" + """ + Send an error message in `ctx` for certain types of CheckFailure. + + The following types are handled: + + * BotMissingPermissions + * BotMissingRole + * BotMissingAnyRole + * NoPrivateMessage + * InChannelCheckFailure + """ bot_missing_errors = ( errors.BotMissingPermissions, errors.BotMissingRole, @@ -151,7 +170,7 @@ class ErrorHandler(Cog): @staticmethod async def handle_api_error(ctx: Context, e: ResponseCodeError) -> None: - """Handle ResponseCodeError exceptions.""" + """Send an error message in `ctx` for ResponseCodeError and log it.""" if e.status == 404: await ctx.send("There does not seem to be anything matching your query.") log.debug(f"API responded with 404 for command {ctx.command}") @@ -168,7 +187,7 @@ class ErrorHandler(Cog): @staticmethod async def handle_unexpected_error(ctx: Context, e: errors.CommandError) -> None: - """Generic handler for errors without an explicit handler.""" + """Send a generic error message in `ctx` and log the exception as an error with exc_info.""" await ctx.send( f"Sorry, an unexpected error occurred. Please let us know!\n\n" f"```{e.__class__.__name__}: {e}```" -- cgit v1.2.3 From b3fdfa71ceeecbd9fe62cadc240fcad27bad7a32 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 16 Feb 2020 19:06:22 -0800 Subject: Error handler: handle CommandOnCooldown errors Simply send the error's default message to the invoking context. --- bot/cogs/error_handler.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index d2c806566..347ce93ae 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -36,9 +36,9 @@ class ErrorHandler(Cog): * Commands in the verification channel are ignored 2. UserInputError: see `handle_user_input_error` 3. CheckFailure: see `handle_check_failure` - 4. ResponseCodeError: see `handle_api_error` - 5. Otherwise, if not a CommandOnCooldown or DisabledCommand, handling is deferred to - `handle_unexpected_error` + 4. CommandOnCooldown: send an error message in the invoking context + 5. ResponseCodeError: see `handle_api_error` + 6. Otherwise, if not a DisabledCommand, handling is deferred to `handle_unexpected_error` """ command = ctx.command @@ -55,13 +55,15 @@ class ErrorHandler(Cog): await self.handle_user_input_error(ctx, e) elif isinstance(e, errors.CheckFailure): await self.handle_check_failure(ctx, e) + elif isinstance(e, errors.CommandOnCooldown): + await ctx.send(e) elif isinstance(e, errors.CommandInvokeError): if isinstance(e.original, ResponseCodeError): await self.handle_api_error(ctx, e.original) else: await self.handle_unexpected_error(ctx, e.original) return # Exit early to avoid logging. - elif not isinstance(e, (errors.CommandOnCooldown, errors.DisabledCommand)): + elif not isinstance(e, errors.DisabledCommand): # ConversionError, MaxConcurrencyReached, ExtensionError await self.handle_unexpected_error(ctx, e) return # Exit early to avoid logging. -- cgit v1.2.3 From 68cf5d8eb9721cb1d91ba004409b82b0a283782d Mon Sep 17 00:00:00 2001 From: Matteo Date: Tue, 18 Feb 2020 17:48:53 +0100 Subject: Use pregenerated partials This avoid recreating partials for each re-eval --- bot/cogs/snekbox.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 3fc8d9937..d075c4fd5 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -203,6 +203,9 @@ class Snekbox(Cog): log.info(f"Received code from {ctx.author} for evaluation:\n{code}") + _predicate_eval_message_edit = partial(predicate_eval_message_edit, ctx) + _predicate_emoji_reaction = partial(predicate_eval_emoji_reaction, ctx) + while True: self.jobs[ctx.author.id] = datetime.datetime.now() code = self.prepare_input(code) @@ -234,13 +237,13 @@ class Snekbox(Cog): try: _, new_message = await self.bot.wait_for( 'message_edit', - check=partial(predicate_eval_message_edit, ctx), + check=_predicate_eval_message_edit, timeout=10 ) await ctx.message.add_reaction('🔁') await self.bot.wait_for( 'reaction_add', - check=partial(predicate_eval_emoji_reaction, ctx), + check=_predicate_emoji_reaction, timeout=10 ) -- cgit v1.2.3 From e1e68ad561513cc02eac0eab59990430fbcfe516 Mon Sep 17 00:00:00 2001 From: Matteo Date: Tue, 18 Feb 2020 18:00:09 +0100 Subject: Suppress HTTPException while deleting bot output It was triggering an error if the user deleted the output before re-evaluating --- bot/cogs/snekbox.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index d075c4fd5..42830fb58 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -1,4 +1,5 @@ import asyncio +import contextlib import datetime import logging import re @@ -7,7 +8,7 @@ from functools import partial from signal import Signals from typing import Optional, Tuple -from discord import Message, Reaction, User +from discord import HTTPException, Message, Reaction, User from discord.ext.commands import Cog, Context, command, guild_only from bot.bot import Bot @@ -250,7 +251,8 @@ class Snekbox(Cog): log.info(f"Re-evaluating message {ctx.message.id}") code = new_message.content.split(' ', maxsplit=1)[1] await ctx.message.clear_reactions() - await response.delete() + with contextlib.suppress(HTTPException): + await response.delete() except asyncio.TimeoutError: await ctx.message.clear_reactions() return -- cgit v1.2.3 From 8b386b533662e7f4be44cc57b9d9c63fde8a7ebf Mon Sep 17 00:00:00 2001 From: Matteo Date: Tue, 18 Feb 2020 18:14:31 +0100 Subject: Snekbox small refactoring Makes the code a bit clearer Co-authored-by: Shirayuki Nekomata --- bot/cogs/snekbox.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 42830fb58..efa4696b5 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -158,8 +158,8 @@ class Snekbox(Cog): lines = output.count("\n") if lines > 0: - output = output.split("\n")[:11] # Only first 11 cause the rest is truncated anyway - output = (f"{i:03d} | {line}" for i, line in enumerate(output, 1)) + output = [f"{i:03d} | {line}" for i, line in enumerate(output.split('\n'), 1)] + output = output[:11] # Limiting to only 11 lines output = "\n".join(output) if lines > 10: @@ -175,8 +175,7 @@ class Snekbox(Cog): if truncated: paste_link = await self.upload_output(original_output) - if not output: - output = "[No output]" + output = output or "[No output]" return output, paste_link -- cgit v1.2.3 From 33769405549efc3c0571cd1b2da5fa0b59ec742a Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Tue, 18 Feb 2020 18:23:38 +0100 Subject: Split assertion onto separate lines Co-Authored-By: Mark --- tests/bot/cogs/test_snekbox.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index 293efed0f..b5190cd7f 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -35,7 +35,8 @@ class SnekboxTests(unittest.TestCase): @async_test async def test_upload_output_reject_too_long(self): """Reject output longer than MAX_PASTE_LEN.""" - self.assertEqual(await self.cog.upload_output("-" * (snekbox.MAX_PASTE_LEN + 1)), "too long to upload") + result = await self.cog.upload_output("-" * (snekbox.MAX_PASTE_LEN + 1)) + self.assertEqual(result, "too long to upload") @async_test async def test_upload_output(self): -- cgit v1.2.3 From 2974d489494370f364c44899a529f5e93c89bedc Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Tue, 18 Feb 2020 18:25:55 +0100 Subject: Split assertions onto separate lines Reads better as separate lines Co-Authored-By: Mark --- tests/bot/cogs/test_snekbox.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index b5190cd7f..01525110d 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -88,7 +88,8 @@ class SnekboxTests(unittest.TestCase): ) for stdout, returncode, expected in cases: with self.subTest(stdout=stdout, returncode=returncode, expected=expected): - self.assertEqual(self.cog.get_results_message({'stdout': stdout, 'returncode': returncode}), expected) + actual = self.cog.get_results_message({'stdout': stdout, 'returncode': returncode}) + self.assertEqual(actual, expected) @patch('bot.cogs.snekbox.Signals', side_effect=ValueError) def test_get_results_message_invalid_signal(self, mock_Signals: Mock): @@ -114,7 +115,8 @@ class SnekboxTests(unittest.TestCase): ) for stdout, returncode, expected in cases: with self.subTest(stdout=stdout, returncode=returncode, expected=expected): - self.assertEqual(self.cog.get_status_emoji({'stdout': stdout, 'returncode': returncode}), expected) + actual = self.cog.get_status_emoji({'stdout': stdout, 'returncode': returncode}) + self.assertEqual(actual, expected) @async_test async def test_format_output(self): @@ -321,7 +323,8 @@ class SnekboxTests(unittest.TestCase): with self.subTest(msg=f'Messages with {testname} return {expected}'): ctx = MockContext() ctx.message = ctx_msg - self.assertEqual(snekbox.predicate_eval_message_edit(ctx, ctx_msg, new_msg), expected) + actual = snekbox.predicate_eval_message_edit(ctx, ctx_msg, new_msg) + self.assertEqual(actual, expected) def test_predicate_eval_emoji_reaction(self): """Test the predicate_eval_emoji_reaction function.""" @@ -351,7 +354,8 @@ class SnekboxTests(unittest.TestCase): ) for reaction, user, expected, testname in cases: with self.subTest(msg=f'Test with {testname} and expected return {expected}'): - self.assertEqual(snekbox.predicate_eval_emoji_reaction(valid_ctx, reaction, user), expected) + actual = snekbox.predicate_eval_emoji_reaction(valid_ctx, reaction, user) + self.assertEqual(actual, expected) class SnekboxSetupTests(unittest.TestCase): -- cgit v1.2.3 From dd9c250253a7bb24751f2de274dfa168efafb717 Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Tue, 18 Feb 2020 18:29:22 +0100 Subject: Delete additional informations from subtest Reduce visual clutter Co-Authored-By: Mark --- tests/bot/cogs/test_snekbox.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index 01525110d..f1f03ab2f 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -76,7 +76,7 @@ class SnekboxTests(unittest.TestCase): ('```py\nprint("Hello world!")```', 'print("Hello world!")', 'multiline python code block'), ) for case, expected, testname in cases: - with self.subTest(msg=f'Extract code from {testname}.', case=case, expected=expected): + with self.subTest(msg=f'Extract code from {testname}.'): self.assertEqual(self.cog.prepare_input(case), expected) def test_get_results_message(self): -- cgit v1.2.3 From b5a1bf7c6ca467cef40a429cc8ca02314526801c Mon Sep 17 00:00:00 2001 From: Matteo Bertucci Date: Tue, 18 Feb 2020 18:30:39 +0100 Subject: Use a space instead of an empty string in test_get_status_emoji Because of the stripping, it should still be considered as empty Co-Authored-By: Mark --- tests/bot/cogs/test_snekbox.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index f1f03ab2f..94ff685c4 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -109,7 +109,7 @@ class SnekboxTests(unittest.TestCase): def test_get_status_emoji(self): """Return emoji according to the eval result.""" cases = ( - ('', -1, ':warning:'), + (' ', -1, ':warning:'), ('Hello world!', 0, ':white_check_mark:'), ('Invalid beard size', -1, ':x:') ) -- cgit v1.2.3 From c1a998ca246e153333438ff91bce200bf74cc0f5 Mon Sep 17 00:00:00 2001 From: Matteo Date: Tue, 18 Feb 2020 18:38:24 +0100 Subject: Assert return value of Snekbox.post_eval --- tests/bot/cogs/test_snekbox.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index 94ff685c4..cf07aad71 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -25,7 +25,9 @@ class SnekboxTests(unittest.TestCase): @async_test async def test_post_eval(self): """Post the eval code to the URLs.snekbox_eval_api endpoint.""" - await self.cog.post_eval("import random") + self.mocked_post.json.return_value = {'lemon': 'AI'} + + self.assertEqual(await self.cog.post_eval("import random"), {'lemon': 'AI'}) self.bot.http_session.post.assert_called_once_with( URLs.snekbox_eval_api, json={"input": "import random"}, -- cgit v1.2.3 From 12f43fc09406dee8cb1b36757fc2af7ae799a9d5 Mon Sep 17 00:00:00 2001 From: Matteo Date: Tue, 18 Feb 2020 18:44:00 +0100 Subject: Use kwargs to set mock attributes --- tests/bot/cogs/test_snekbox.py | 36 +++++++++++------------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index cf07aad71..112c923c8 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -306,15 +306,9 @@ class SnekboxTests(unittest.TestCase): def test_predicate_eval_message_edit(self): """Test the predicate_eval_message_edit function.""" - msg0 = MockMessage() - msg0.id = 1 - msg0.content = 'abc' - msg1 = MockMessage() - msg1.id = 2 - msg1.content = 'abcdef' - msg2 = MockMessage() - msg2.id = 1 - msg2.content = 'abcdef' + msg0 = MockMessage(id=1, content='abc') + msg1 = MockMessage(id=2, content='abcdef') + msg2 = MockMessage(id=1, content='abcdef') cases = ( (msg0, msg0, False, 'same ID, same content'), @@ -323,29 +317,21 @@ class SnekboxTests(unittest.TestCase): ) for ctx_msg, new_msg, expected, testname in cases: with self.subTest(msg=f'Messages with {testname} return {expected}'): - ctx = MockContext() - ctx.message = ctx_msg + ctx = MockContext(message=ctx_msg) actual = snekbox.predicate_eval_message_edit(ctx, ctx_msg, new_msg) self.assertEqual(actual, expected) def test_predicate_eval_emoji_reaction(self): """Test the predicate_eval_emoji_reaction function.""" - valid_reaction = MockReaction() - valid_reaction.message.id = 1 + valid_reaction = MockReaction(message=MockMessage(id=1)) valid_reaction.__str__.return_value = '🔁' - valid_ctx = MockContext() - valid_ctx.message.id = 1 - valid_ctx.author.id = 2 - valid_user = MockUser() - valid_user.id = 2 - - invalid_reaction_id = MockReaction() - invalid_reaction_id.message.id = 42 + valid_ctx = MockContext(message=MockMessage(id=1), author=MockUser(id=2)) + valid_user = MockUser(id=2) + + invalid_reaction_id = MockReaction(message=MockMessage(id=42)) invalid_reaction_id.__str__.return_value = '🔁' - invalid_user_id = MockUser() - invalid_user_id.id = 42 - invalid_reaction_str = MockReaction() - invalid_reaction_str.message.id = 1 + invalid_user_id = MockUser(id=42) + invalid_reaction_str = MockReaction(message=MockMessage(id=1)) invalid_reaction_str.__str__.return_value = ':longbeard:' cases = ( -- cgit v1.2.3 From f91c32fed74bd5daebb8438c79f4d2d9efbc1459 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 09:59:28 -0800 Subject: Reminders: don't cancel task if reminder is invalid when rescheduling If a reminder is invalid, it won't get rescheduled. Therefore, there wouldn't exist a task to cancel and it'd raise a warning. Fixes BOT-1C --- bot/cogs/reminders.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/bot/cogs/reminders.py b/bot/cogs/reminders.py index ef46f4f3e..f3e516158 100644 --- a/bot/cogs/reminders.py +++ b/bot/cogs/reminders.py @@ -45,7 +45,7 @@ class Reminders(Scheduler, Cog): loop = asyncio.get_event_loop() for reminder in response: - is_valid, *_ = self.ensure_valid_reminder(reminder) + is_valid, *_ = self.ensure_valid_reminder(reminder, cancel_task=False) if not is_valid: continue @@ -59,7 +59,11 @@ class Reminders(Scheduler, Cog): else: self.schedule_task(loop, reminder["id"], reminder) - def ensure_valid_reminder(self, reminder: dict) -> t.Tuple[bool, discord.User, discord.TextChannel]: + def ensure_valid_reminder( + self, + reminder: dict, + cancel_task: bool = True + ) -> t.Tuple[bool, discord.User, discord.TextChannel]: """Ensure reminder author and channel can be fetched otherwise delete the reminder.""" user = self.bot.get_user(reminder['author']) channel = self.bot.get_channel(reminder['channel_id']) @@ -70,7 +74,7 @@ class Reminders(Scheduler, Cog): f"Reminder {reminder['id']} invalid: " f"User {reminder['author']}={user}, Channel {reminder['channel_id']}={channel}." ) - asyncio.create_task(self._delete_reminder(reminder['id'])) + asyncio.create_task(self._delete_reminder(reminder['id'], cancel_task)) return is_valid, user, channel @@ -98,12 +102,13 @@ class Reminders(Scheduler, Cog): # Now we can begone with it from our schedule list. self.cancel_task(reminder_id) - async def _delete_reminder(self, reminder_id: str) -> None: + async def _delete_reminder(self, reminder_id: str, cancel_task: bool = True) -> None: """Delete a reminder from the database, given its ID, and cancel the running task.""" await self.bot.api_client.delete('bot/reminders/' + str(reminder_id)) - # Now we can remove it from the schedule list - self.cancel_task(reminder_id) + if cancel_task: + # Now we can remove it from the schedule list + self.cancel_task(reminder_id) async def _reschedule_reminder(self, reminder: dict) -> None: """Reschedule a reminder object.""" -- cgit v1.2.3 From f12d76dca1073b489b4a407a17dbab623aa0dce5 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:18:01 -0800 Subject: Config: rename roles to match their names in the guild --- config-default.yml | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/config-default.yml b/config-default.yml index 379475907..5755b2191 100644 --- a/config-default.yml +++ b/config-default.yml @@ -160,20 +160,20 @@ guild: reminder_whitelist: [*BOT_CMD, *DEV_CONTRIB] roles: - admin: &ADMIN_ROLE 267628507062992896 - announcements: 463658397560995840 - champion: 430492892331769857 - contributor: 295488872404484098 - core_developer: 587606783669829632 - helpers: 267630620367257601 - jammer: 591786436651646989 - moderator: &MOD_ROLE 267629731250176001 - muted: &MUTED_ROLE 277914926603829249 - owner: &OWNER_ROLE 267627879762755584 - partners: 323426753857191936 - rockstars: &ROCKSTARS_ROLE 458226413825294336 - team_leader: 501324292341104650 - verified: 352427296948486144 + admins: &ADMINS_ROLE 267628507062992896 + announcements: 463658397560995840 + code_jam_champions: 430492892331769857 + contributors: 295488872404484098 + core_developers: 587606783669829632 + developers: 352427296948486144 + helpers: 267630620367257601 + jammers: 591786436651646989 + moderators: &MODS_ROLE 267629731250176001 + muted: &MUTED_ROLE 277914926603829249 + owners: &OWNERS_ROLE 267627879762755584 + partners: 323426753857191936 + python_community: &PYTHON_COMMUNITY_ROLE 458226413825294336 + team_leaders: 501324292341104650 webhooks: talent_pool: 569145364800602132 @@ -267,10 +267,10 @@ filter: - *USER_EVENT_A role_whitelist: - - *ADMIN_ROLE - - *MOD_ROLE - - *OWNER_ROLE - - *ROCKSTARS_ROLE + - *ADMINS_ROLE + - *MODS_ROLE + - *OWNERS_ROLE + - *PYTHON_COMMUNITY_ROLE keys: -- cgit v1.2.3 From ffdde0f8a2f14590baf47462143fbd685d851fad Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:20:54 -0800 Subject: Config: split roles into categories --- config-default.yml | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/config-default.yml b/config-default.yml index 5755b2191..203f9e64e 100644 --- a/config-default.yml +++ b/config-default.yml @@ -160,26 +160,30 @@ guild: reminder_whitelist: [*BOT_CMD, *DEV_CONTRIB] roles: - admins: &ADMINS_ROLE 267628507062992896 announcements: 463658397560995840 - code_jam_champions: 430492892331769857 contributors: 295488872404484098 - core_developers: 587606783669829632 developers: 352427296948486144 - helpers: 267630620367257601 - jammers: 591786436651646989 - moderators: &MODS_ROLE 267629731250176001 muted: &MUTED_ROLE 277914926603829249 - owners: &OWNERS_ROLE 267627879762755584 partners: 323426753857191936 python_community: &PYTHON_COMMUNITY_ROLE 458226413825294336 - team_leaders: 501324292341104650 + + # Staff + admins: &ADMINS_ROLE 267628507062992896 + core_developers: 587606783669829632 + helpers: 267630620367257601 + moderators: &MODS_ROLE 267629731250176001 + owners: &OWNERS_ROLE 267627879762755584 + + # Code Jam + code_jam_champions: 430492892331769857 + jammers: 591786436651646989 + team_leaders: 501324292341104650 webhooks: - talent_pool: 569145364800602132 - big_brother: 569133704568373283 - reddit: 635408384794951680 - duck_pond: 637821475327311927 + talent_pool: 569145364800602132 + big_brother: 569133704568373283 + reddit: 635408384794951680 + duck_pond: 637821475327311927 filter: -- cgit v1.2.3 From bf7dd8c17dea3d0c12139893c210500fdad820f9 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:46:12 -0800 Subject: Config: split channels into categories --- config-default.yml | 60 +++++++++++++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/config-default.yml b/config-default.yml index 203f9e64e..415ed13f4 100644 --- a/config-default.yml +++ b/config-default.yml @@ -113,19 +113,32 @@ guild: python_help: 356013061213126657 channels: - admins: &ADMINS 365960823622991872 - admin_spam: &ADMIN_SPAM 563594791770914816 - admins_voice: &ADMINS_VOICE 500734494840717332 announcements: 354619224620138496 - attachment_log: &ATTCH_LOG 649243850006855680 - big_brother_logs: &BBLOGS 468507907357409333 - bot: &BOT_CMD 267659945086812160 checkpoint_test: 422077681434099723 - defcon: &DEFCON 464469101889454091 + user_event_a: &USER_EVENT_A 592000283102674944 + + # Development devcontrib: &DEV_CONTRIB 635950537262759947 devlog: &DEVLOG 622895325144940554 devtest: &DEVTEST 414574275865870337 - esoteric: 470884583684964352 + + # Discussion + meta: 429409067623251969 + python: 267624335836053506 + + # Logs + attachment_log: &ATTCH_LOG 649243850006855680 + message_log: &MESSAGE_LOG 467752170159079424 + modlog: &MODLOG 282638479504965634 + userlog: 528976905546760203 + voice_log: 640292421988646961 + + # Off-topic + off_topic_0: 291284109232308226 + off_topic_1: 463035241142026251 + off_topic_2: 463035268514185226 + + # Python Help help_0: 303906576991780866 help_1: 303906556754395136 help_2: 303906514266226689 @@ -134,26 +147,31 @@ guild: help_5: 454941769734422538 help_6: 587375753306570782 help_7: 587375768556797982 + + # Special + bot: &BOT_CMD 267659945086812160 + esoteric: 470884583684964352 + reddit: 458224812528238616 + verification: 352442727016693763 + + # Staff + admins: &ADMINS 365960823622991872 + admin_spam: &ADMIN_SPAM 563594791770914816 + defcon: &DEFCON 464469101889454091 helpers: &HELPERS 385474242440986624 - message_log: &MESSAGE_LOG 467752170159079424 - meta: 429409067623251969 - mod_spam: &MOD_SPAM 620607373828030464 mods: &MODS 305126844661760000 mod_alerts: 473092532147060736 - modlog: &MODLOG 282638479504965634 - off_topic_0: 291284109232308226 - off_topic_1: 463035241142026251 - off_topic_2: 463035268514185226 + mod_spam: &MOD_SPAM 620607373828030464 organisation: &ORGANISATION 551789653284356126 - python: 267624335836053506 - reddit: 458224812528238616 staff_lounge: &STAFF_LOUNGE 464905259261755392 + + # Voice + admins_voice: &ADMINS_VOICE 500734494840717332 staff_voice: &STAFF_VOICE 412375055910043655 + + # Watch + big_brother_logs: &BBLOGS 468507907357409333 talent_pool: &TALENT_POOL 534321732593647616 - userlog: 528976905546760203 - user_event_a: &USER_EVENT_A 592000283102674944 - verification: 352442727016693763 - voice_log: 640292421988646961 staff_channels: [*ADMINS, *ADMIN_SPAM, *MOD_SPAM, *MODS, *HELPERS, *ORGANISATION, *DEFCON] ignored: [*ADMINS, *MESSAGE_LOG, *MODLOG, *ADMINS_VOICE, *STAFF_VOICE, *ATTCH_LOG] -- cgit v1.2.3 From 707d2f7208e7008f94bfff1a1e13664dc38c6d6c Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:47:11 -0800 Subject: Config: shorten name of PYTHON_COMMUNITY_ROLE --- config-default.yml | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/config-default.yml b/config-default.yml index 415ed13f4..1e478154f 100644 --- a/config-default.yml +++ b/config-default.yml @@ -178,12 +178,12 @@ guild: reminder_whitelist: [*BOT_CMD, *DEV_CONTRIB] roles: - announcements: 463658397560995840 - contributors: 295488872404484098 - developers: 352427296948486144 - muted: &MUTED_ROLE 277914926603829249 - partners: 323426753857191936 - python_community: &PYTHON_COMMUNITY_ROLE 458226413825294336 + announcements: 463658397560995840 + contributors: 295488872404484098 + developers: 352427296948486144 + muted: &MUTED_ROLE 277914926603829249 + partners: 323426753857191936 + python_community: &PY_COMMUNITY_ROLE 458226413825294336 # Staff admins: &ADMINS_ROLE 267628507062992896 @@ -292,7 +292,7 @@ filter: - *ADMINS_ROLE - *MODS_ROLE - *OWNERS_ROLE - - *PYTHON_COMMUNITY_ROLE + - *PY_COMMUNITY_ROLE keys: -- cgit v1.2.3 From 8c1a31b7d043facd876b22800e8cb44ae15d9492 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:50:21 -0800 Subject: Config: remove checkpoint_test and devtest They no longer exist in the guild. * Move devlog under the "Logs" category --- bot/cogs/bot.py | 1 - bot/cogs/tags.py | 1 - bot/constants.py | 2 -- config-default.yml | 2 -- 4 files changed, 6 deletions(-) diff --git a/bot/cogs/bot.py b/bot/cogs/bot.py index 73b1e8f41..74e882e0e 100644 --- a/bot/cogs/bot.py +++ b/bot/cogs/bot.py @@ -40,7 +40,6 @@ class BotCog(Cog, name="Bot"): # These channels will also work, but will not be subject to cooldown self.channel_whitelist = ( Channels.bot, - Channels.devtest, ) # Stores improperly formatted Python codeblock message ids and the corresponding bot message diff --git a/bot/cogs/tags.py b/bot/cogs/tags.py index b6360dfae..a38f5617f 100644 --- a/bot/cogs/tags.py +++ b/bot/cogs/tags.py @@ -15,7 +15,6 @@ from bot.pagination import LinePaginator log = logging.getLogger(__name__) TEST_CHANNELS = ( - Channels.devtest, Channels.bot, Channels.helpers ) diff --git a/bot/constants.py b/bot/constants.py index 681d8da49..15f078cbf 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -363,11 +363,9 @@ class Channels(metaclass=YAMLGetter): attachment_log: int big_brother_logs: int bot: int - checkpoint_test: int defcon: int devcontrib: int devlog: int - devtest: int esoteric: int help_0: int help_1: int diff --git a/config-default.yml b/config-default.yml index 1e478154f..058317262 100644 --- a/config-default.yml +++ b/config-default.yml @@ -114,13 +114,11 @@ guild: channels: announcements: 354619224620138496 - checkpoint_test: 422077681434099723 user_event_a: &USER_EVENT_A 592000283102674944 # Development devcontrib: &DEV_CONTRIB 635950537262759947 devlog: &DEVLOG 622895325144940554 - devtest: &DEVTEST 414574275865870337 # Discussion meta: 429409067623251969 -- cgit v1.2.3 From a850a32135ef3b454abfe2ad017a46badd73d3c1 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:54:58 -0800 Subject: Constants: rename roles to match their names in the guild --- bot/cogs/defcon.py | 12 ++++++------ bot/cogs/eval.py | 4 ++-- bot/cogs/extensions.py | 2 +- bot/cogs/jams.py | 6 +++--- bot/cogs/moderation/scheduler.py | 2 +- bot/cogs/snekbox.py | 2 +- bot/cogs/tags.py | 2 +- bot/cogs/verification.py | 2 +- bot/constants.py | 22 +++++++++++----------- tests/bot/cogs/test_information.py | 2 +- 10 files changed, 28 insertions(+), 28 deletions(-) diff --git a/bot/cogs/defcon.py b/bot/cogs/defcon.py index a0d8fedd5..c7ea1f2bf 100644 --- a/bot/cogs/defcon.py +++ b/bot/cogs/defcon.py @@ -69,7 +69,7 @@ class Defcon(Cog): except Exception: # Yikes! log.exception("Unable to get DEFCON settings!") await self.bot.get_channel(Channels.devlog).send( - f"<@&{Roles.admin}> **WARNING**: Unable to get DEFCON settings!" + f"<@&{Roles.admins}> **WARNING**: Unable to get DEFCON settings!" ) else: @@ -118,7 +118,7 @@ class Defcon(Cog): ) @group(name='defcon', aliases=('dc',), invoke_without_command=True) - @with_role(Roles.admin, Roles.owner) + @with_role(Roles.admins, Roles.owners) async def defcon_group(self, ctx: Context) -> None: """Check the DEFCON status or run a subcommand.""" await ctx.invoke(self.bot.get_command("help"), "defcon") @@ -146,7 +146,7 @@ class Defcon(Cog): await self.send_defcon_log(action, ctx.author, error) @defcon_group.command(name='enable', aliases=('on', 'e')) - @with_role(Roles.admin, Roles.owner) + @with_role(Roles.admins, Roles.owners) async def enable_command(self, ctx: Context) -> None: """ Enable DEFCON mode. Useful in a pinch, but be sure you know what you're doing! @@ -159,7 +159,7 @@ class Defcon(Cog): await self.update_channel_topic() @defcon_group.command(name='disable', aliases=('off', 'd')) - @with_role(Roles.admin, Roles.owner) + @with_role(Roles.admins, Roles.owners) async def disable_command(self, ctx: Context) -> None: """Disable DEFCON mode. Useful in a pinch, but be sure you know what you're doing!""" self.enabled = False @@ -167,7 +167,7 @@ class Defcon(Cog): await self.update_channel_topic() @defcon_group.command(name='status', aliases=('s',)) - @with_role(Roles.admin, Roles.owner) + @with_role(Roles.admins, Roles.owners) async def status_command(self, ctx: Context) -> None: """Check the current status of DEFCON mode.""" embed = Embed( @@ -179,7 +179,7 @@ class Defcon(Cog): await ctx.send(embed=embed) @defcon_group.command(name='days') - @with_role(Roles.admin, Roles.owner) + @with_role(Roles.admins, Roles.owners) async def days_command(self, ctx: Context, days: int) -> None: """Set how old an account must be to join the server, in days, with DEFCON mode enabled.""" self.days = timedelta(days=days) diff --git a/bot/cogs/eval.py b/bot/cogs/eval.py index 9c729f28a..52136fc8d 100644 --- a/bot/cogs/eval.py +++ b/bot/cogs/eval.py @@ -174,14 +174,14 @@ async def func(): # (None,) -> Any await ctx.send(f"```py\n{out}```", embed=embed) @group(name='internal', aliases=('int',)) - @with_role(Roles.owner, Roles.admin) + @with_role(Roles.owners, Roles.admins) async def internal_group(self, ctx: Context) -> None: """Internal commands. Top secret!""" if not ctx.invoked_subcommand: await ctx.invoke(self.bot.get_command("help"), "internal") @internal_group.command(name='eval', aliases=('e',)) - @with_role(Roles.admin, Roles.owner) + @with_role(Roles.admins, Roles.owners) async def eval(self, ctx: Context, *, code: str) -> None: """Run eval in a REPL-like format.""" code = code.strip("`") diff --git a/bot/cogs/extensions.py b/bot/cogs/extensions.py index f16e79fb7..b312e1a1d 100644 --- a/bot/cogs/extensions.py +++ b/bot/cogs/extensions.py @@ -221,7 +221,7 @@ class Extensions(commands.Cog): # This cannot be static (must have a __func__ attribute). def cog_check(self, ctx: Context) -> bool: """Only allow moderators and core developers to invoke the commands in this cog.""" - return with_role_check(ctx, *MODERATION_ROLES, Roles.core_developer) + return with_role_check(ctx, *MODERATION_ROLES, Roles.core_developers) # This cannot be static (must have a __func__ attribute). async def cog_command_error(self, ctx: Context, error: Exception) -> None: diff --git a/bot/cogs/jams.py b/bot/cogs/jams.py index 985f28ce5..1d062b0c2 100644 --- a/bot/cogs/jams.py +++ b/bot/cogs/jams.py @@ -18,7 +18,7 @@ class CodeJams(commands.Cog): self.bot = bot @commands.command() - @with_role(Roles.admin) + @with_role(Roles.admins) async def createteam(self, ctx: commands.Context, team_name: str, members: commands.Greedy[Member]) -> None: """ Create team channels (voice and text) in the Code Jams category, assign roles, and add overwrites for the team. @@ -95,10 +95,10 @@ class CodeJams(commands.Cog): ) # Assign team leader role - await members[0].add_roles(ctx.guild.get_role(Roles.team_leader)) + await members[0].add_roles(ctx.guild.get_role(Roles.team_leaders)) # Assign rest of roles - jammer_role = ctx.guild.get_role(Roles.jammer) + jammer_role = ctx.guild.get_role(Roles.jammers) for member in members: await member.add_roles(jammer_role) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index c0de0e4da..db1a3030e 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -307,7 +307,7 @@ class InfractionScheduler(Scheduler): Infractions of unsupported types will raise a ValueError. """ guild = self.bot.get_guild(constants.Guild.id) - mod_role = guild.get_role(constants.Roles.moderator) + mod_role = guild.get_role(constants.Roles.moderators) user_id = infraction["user"] actor = infraction["actor"] type_ = infraction["type"] diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index da33e27b2..84457e38f 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -34,7 +34,7 @@ RAW_CODE_REGEX = re.compile( ) MAX_PASTE_LEN = 1000 -EVAL_ROLES = (Roles.helpers, Roles.moderator, Roles.admin, Roles.owner, Roles.rockstars, Roles.partners) +EVAL_ROLES = (Roles.helpers, Roles.moderators, Roles.admins, Roles.owners, Roles.python_community, Roles.partners) class Snekbox(Cog): diff --git a/bot/cogs/tags.py b/bot/cogs/tags.py index a38f5617f..2c4fa02bd 100644 --- a/bot/cogs/tags.py +++ b/bot/cogs/tags.py @@ -220,7 +220,7 @@ class Tags(Cog): )) @tags_group.command(name='delete', aliases=('remove', 'rm', 'd')) - @with_role(Roles.admin, Roles.owner) + @with_role(Roles.admins, Roles.owners) async def delete_command(self, ctx: Context, *, tag_name: TagNameConverter) -> None: """Remove a tag from the database.""" await self.bot.api_client.delete(f'bot/tags/{tag_name}') diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 582237374..09bef80c4 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -38,7 +38,7 @@ If you'd like to unsubscribe from the announcement notifications, simply send `! PERIODIC_PING = ( f"@everyone To verify that you have read our rules, please type `{BotConfig.prefix}accept`." - f" If you encounter any problems during the verification process, ping the <@&{Roles.admin}> role in this channel." + f" If you encounter any problems during the verification process, ping the <@&{Roles.admins}> role in this channel." ) BOT_MESSAGE_DELETE_DELAY = 10 diff --git a/bot/constants.py b/bot/constants.py index 15f078cbf..03578fefd 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -409,19 +409,19 @@ class Roles(metaclass=YAMLGetter): section = "guild" subsection = "roles" - admin: int + admins: int announcements: int - champion: int - contributor: int - core_developer: int + code_jam_champions: int + contributors: int + core_developers: int helpers: int - jammer: int - moderator: int + jammers: int + moderators: int muted: int - owner: int + owners: int partners: int - rockstars: int - team_leader: int + python_community: int + team_leaders: int verified: int # This is the Developers role on PyDis, here named verified for readability reasons. @@ -570,8 +570,8 @@ BOT_DIR = os.path.dirname(__file__) PROJECT_ROOT = os.path.abspath(os.path.join(BOT_DIR, os.pardir)) # Default role combinations -MODERATION_ROLES = Roles.moderator, Roles.admin, Roles.owner -STAFF_ROLES = Roles.helpers, Roles.moderator, Roles.admin, Roles.owner +MODERATION_ROLES = Roles.moderators, Roles.admins, Roles.owners +STAFF_ROLES = Roles.helpers, Roles.moderators, Roles.admins, Roles.owners # Roles combinations STAFF_CHANNELS = Guild.staff_channels diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index deae7ebad..38293269f 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -19,7 +19,7 @@ class InformationCogTests(unittest.TestCase): @classmethod def setUpClass(cls): - cls.moderator_role = helpers.MockRole(name="Moderator", id=constants.Roles.moderator) + cls.moderator_role = helpers.MockRole(name="Moderator", id=constants.Roles.moderators) def setUp(self): """Sets up fresh objects for each test.""" -- cgit v1.2.3 From ed25a7ae240cce1162a0a67fa6451363671cc7de Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:58:51 -0800 Subject: Constants: rename developers role back to verified It makes the code which uses it more readable. A comment was added to explain the discrepancy between the constant's name and the name in the guild. --- config-default.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/config-default.yml b/config-default.yml index 058317262..dc5be5d47 100644 --- a/config-default.yml +++ b/config-default.yml @@ -178,11 +178,13 @@ guild: roles: announcements: 463658397560995840 contributors: 295488872404484098 - developers: 352427296948486144 muted: &MUTED_ROLE 277914926603829249 partners: 323426753857191936 python_community: &PY_COMMUNITY_ROLE 458226413825294336 + # This is the Developers role on PyDis, here named verified for readability reasons + verified: 352427296948486144 + # Staff admins: &ADMINS_ROLE 267628507062992896 core_developers: 587606783669829632 -- cgit v1.2.3 From e38990dc26fd77323e33da267ab8f16538f32438 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 11:59:38 -0800 Subject: Constants: remove code jam champions role Nothing was using it. --- bot/constants.py | 1 - config-default.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/bot/constants.py b/bot/constants.py index 03578fefd..98914fb9d 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -411,7 +411,6 @@ class Roles(metaclass=YAMLGetter): admins: int announcements: int - code_jam_champions: int contributors: int core_developers: int helpers: int diff --git a/config-default.yml b/config-default.yml index dc5be5d47..10cc34816 100644 --- a/config-default.yml +++ b/config-default.yml @@ -193,7 +193,6 @@ guild: owners: &OWNERS_ROLE 267627879762755584 # Code Jam - code_jam_champions: 430492892331769857 jammers: 591786436651646989 team_leaders: 501324292341104650 -- cgit v1.2.3 From 734fd8ff1a0e5bc309b0d84e34b2b6a1d0c204d9 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 12:16:32 -0800 Subject: Config: rename channels to match their names in the guild --- bot/cogs/bot.py | 4 +- bot/cogs/clean.py | 2 +- bot/cogs/defcon.py | 2 +- bot/cogs/free.py | 2 +- bot/cogs/help.py | 2 +- bot/cogs/information.py | 6 +-- bot/cogs/logging.py | 2 +- bot/cogs/moderation/modlog.py | 12 +++--- bot/cogs/snekbox.py | 2 +- bot/cogs/tags.py | 2 +- bot/cogs/utils.py | 2 +- bot/cogs/verification.py | 9 +++-- bot/constants.py | 12 +++--- config-default.yml | 93 +++++++++++++++++++++---------------------- 14 files changed, 76 insertions(+), 76 deletions(-) diff --git a/bot/cogs/bot.py b/bot/cogs/bot.py index 74e882e0e..f17135877 100644 --- a/bot/cogs/bot.py +++ b/bot/cogs/bot.py @@ -34,12 +34,12 @@ class BotCog(Cog, name="Bot"): Channels.help_5: 0, Channels.help_6: 0, Channels.help_7: 0, - Channels.python: 0, + Channels.python_discussion: 0, } # These channels will also work, but will not be subject to cooldown self.channel_whitelist = ( - Channels.bot, + Channels.bot_commands, ) # Stores improperly formatted Python codeblock message ids and the corresponding bot message diff --git a/bot/cogs/clean.py b/bot/cogs/clean.py index 2104efe57..5cdf0b048 100644 --- a/bot/cogs/clean.py +++ b/bot/cogs/clean.py @@ -173,7 +173,7 @@ class Clean(Cog): colour=Colour(Colours.soft_red), title="Bulk message delete", text=message, - channel_id=Channels.modlog, + channel_id=Channels.mod_log, ) @group(invoke_without_command=True, name="clean", aliases=["purge"]) diff --git a/bot/cogs/defcon.py b/bot/cogs/defcon.py index c7ea1f2bf..050760a71 100644 --- a/bot/cogs/defcon.py +++ b/bot/cogs/defcon.py @@ -68,7 +68,7 @@ class Defcon(Cog): except Exception: # Yikes! log.exception("Unable to get DEFCON settings!") - await self.bot.get_channel(Channels.devlog).send( + await self.bot.get_channel(Channels.dev_log).send( f"<@&{Roles.admins}> **WARNING**: Unable to get DEFCON settings!" ) diff --git a/bot/cogs/free.py b/bot/cogs/free.py index 49cab6172..02c02d067 100644 --- a/bot/cogs/free.py +++ b/bot/cogs/free.py @@ -22,7 +22,7 @@ class Free(Cog): PYTHON_HELP_ID = Categories.python_help @command(name="free", aliases=('f',)) - @redirect_output(destination_channel=Channels.bot, bypass_roles=STAFF_ROLES) + @redirect_output(destination_channel=Channels.bot_commands, bypass_roles=STAFF_ROLES) async def free(self, ctx: Context, user: Member = None, seek: int = 2) -> None: """ Lists free help channels by likeliness of availability. diff --git a/bot/cogs/help.py b/bot/cogs/help.py index fd5bbc3ca..744722220 100644 --- a/bot/cogs/help.py +++ b/bot/cogs/help.py @@ -507,7 +507,7 @@ class Help(DiscordCog): """Custom Embed Pagination Help feature.""" @commands.command('help') - @redirect_output(destination_channel=Channels.bot, bypass_roles=STAFF_ROLES) + @redirect_output(destination_channel=Channels.bot_commands, bypass_roles=STAFF_ROLES) async def new_help(self, ctx: Context, *commands) -> None: """Shows Command Help.""" try: diff --git a/bot/cogs/information.py b/bot/cogs/information.py index 13c8aabaa..49beca15b 100644 --- a/bot/cogs/information.py +++ b/bot/cogs/information.py @@ -152,8 +152,8 @@ class Information(Cog): # Non-staff may only do this in #bot-commands if not with_role_check(ctx, *constants.STAFF_ROLES): - if not ctx.channel.id == constants.Channels.bot: - raise InChannelCheckFailure(constants.Channels.bot) + if not ctx.channel.id == constants.Channels.bot_commands: + raise InChannelCheckFailure(constants.Channels.bot_commands) embed = await self.create_user_embed(ctx, user) @@ -332,7 +332,7 @@ class Information(Cog): @cooldown_with_role_bypass(2, 60 * 3, BucketType.member, bypass_roles=constants.STAFF_ROLES) @group(invoke_without_command=True) - @in_channel(constants.Channels.bot, bypass_roles=constants.STAFF_ROLES) + @in_channel(constants.Channels.bot_commands, bypass_roles=constants.STAFF_ROLES) async def raw(self, ctx: Context, *, message: Message, json: bool = False) -> None: """Shows information about the raw API response.""" # I *guess* it could be deleted right as the command is invoked but I felt like it wasn't worth handling diff --git a/bot/cogs/logging.py b/bot/cogs/logging.py index d1b7dcab3..9dcb1456b 100644 --- a/bot/cogs/logging.py +++ b/bot/cogs/logging.py @@ -34,7 +34,7 @@ class Logging(Cog): ) if not DEBUG_MODE: - await self.bot.get_channel(Channels.devlog).send(embed=embed) + await self.bot.get_channel(Channels.dev_log).send(embed=embed) def setup(bot: Bot) -> None: diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index e8ae0dbe6..94e646248 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -87,7 +87,7 @@ class ModLog(Cog, name="ModLog"): title: t.Optional[str], text: str, thumbnail: t.Optional[t.Union[str, discord.Asset]] = None, - channel_id: int = Channels.modlog, + channel_id: int = Channels.mod_log, ping_everyone: bool = False, files: t.Optional[t.List[discord.File]] = None, content: t.Optional[str] = None, @@ -377,7 +377,7 @@ class ModLog(Cog, name="ModLog"): Icons.user_ban, Colours.soft_red, "User banned", f"{member} (`{member.id}`)", thumbnail=member.avatar_url_as(static_format="png"), - channel_id=Channels.userlog + channel_id=Channels.user_log ) @Cog.listener() @@ -399,7 +399,7 @@ class ModLog(Cog, name="ModLog"): Icons.sign_in, Colours.soft_green, "User joined", message, thumbnail=member.avatar_url_as(static_format="png"), - channel_id=Channels.userlog + channel_id=Channels.user_log ) @Cog.listener() @@ -416,7 +416,7 @@ class ModLog(Cog, name="ModLog"): Icons.sign_out, Colours.soft_red, "User left", f"{member} (`{member.id}`)", thumbnail=member.avatar_url_as(static_format="png"), - channel_id=Channels.userlog + channel_id=Channels.user_log ) @Cog.listener() @@ -433,7 +433,7 @@ class ModLog(Cog, name="ModLog"): Icons.user_unban, Colour.blurple(), "User unbanned", f"{member} (`{member.id}`)", thumbnail=member.avatar_url_as(static_format="png"), - channel_id=Channels.modlog + channel_id=Channels.mod_log ) @Cog.listener() @@ -529,7 +529,7 @@ class ModLog(Cog, name="ModLog"): Icons.user_update, Colour.blurple(), "Member updated", message, thumbnail=after.avatar_url_as(static_format="png"), - channel_id=Channels.userlog + channel_id=Channels.user_log ) @Cog.listener() diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 84457e38f..aef12546d 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -177,7 +177,7 @@ class Snekbox(Cog): @command(name="eval", aliases=("e",)) @guild_only() - @in_channel(Channels.bot, hidden_channels=(Channels.esoteric,), bypass_roles=EVAL_ROLES) + @in_channel(Channels.bot_commands, hidden_channels=(Channels.esoteric,), bypass_roles=EVAL_ROLES) async def eval_command(self, ctx: Context, *, code: str = None) -> None: """ Run Python code and get the results. diff --git a/bot/cogs/tags.py b/bot/cogs/tags.py index 2c4fa02bd..5da9a4148 100644 --- a/bot/cogs/tags.py +++ b/bot/cogs/tags.py @@ -15,7 +15,7 @@ from bot.pagination import LinePaginator log = logging.getLogger(__name__) TEST_CHANNELS = ( - Channels.bot, + Channels.bot_commands, Channels.helpers ) diff --git a/bot/cogs/utils.py b/bot/cogs/utils.py index da278011a..94b9d6b5a 100644 --- a/bot/cogs/utils.py +++ b/bot/cogs/utils.py @@ -89,7 +89,7 @@ class Utils(Cog): await ctx.message.channel.send(embed=pep_embed) @command() - @in_channel(Channels.bot, bypass_roles=STAFF_ROLES) + @in_channel(Channels.bot_commands, bypass_roles=STAFF_ROLES) async def charinfo(self, ctx: Context, *, characters: str) -> None: """Shows you information on up to 25 unicode characters.""" match = re.match(r"<(a?):(\w+):(\d+)>", characters) diff --git a/bot/cogs/verification.py b/bot/cogs/verification.py index 09bef80c4..94bef3188 100644 --- a/bot/cogs/verification.py +++ b/bot/cogs/verification.py @@ -30,10 +30,11 @@ your information removed here as well. Feel free to review them at any point! Additionally, if you'd like to receive notifications for the announcements we post in <#{Channels.announcements}> \ -from time to time, you can send `!subscribe` to <#{Channels.bot}> at any time to assign yourself the \ +from time to time, you can send `!subscribe` to <#{Channels.bot_commands}> at any time to assign yourself the \ **Announcements** role. We'll mention this role every time we make an announcement. -If you'd like to unsubscribe from the announcement notifications, simply send `!unsubscribe` to <#{Channels.bot}>. +If you'd like to unsubscribe from the announcement notifications, simply send `!unsubscribe` to \ +<#{Channels.bot_commands}>. """ PERIODIC_PING = ( @@ -136,7 +137,7 @@ class Verification(Cog): await ctx.message.delete() @command(name='subscribe') - @in_channel(Channels.bot) + @in_channel(Channels.bot_commands) async def subscribe_command(self, ctx: Context, *_) -> None: # We don't actually care about the args """Subscribe to announcement notifications by assigning yourself the role.""" has_role = False @@ -160,7 +161,7 @@ class Verification(Cog): ) @command(name='unsubscribe') - @in_channel(Channels.bot) + @in_channel(Channels.bot_commands) async def unsubscribe_command(self, ctx: Context, *_) -> None: # We don't actually care about the args """Unsubscribe from announcement notifications by removing the role from yourself.""" has_role = False diff --git a/bot/constants.py b/bot/constants.py index 98914fb9d..f35d608da 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -362,10 +362,10 @@ class Channels(metaclass=YAMLGetter): announcements: int attachment_log: int big_brother_logs: int - bot: int + bot_commands: int defcon: int devcontrib: int - devlog: int + dev_log: int esoteric: int help_0: int help_1: int @@ -381,16 +381,16 @@ class Channels(metaclass=YAMLGetter): mod_spam: int mods: int mod_alerts: int - modlog: int + mod_log: int off_topic_0: int off_topic_1: int off_topic_2: int organisation: int - python: int + python_discussion: int reddit: int talent_pool: int - userlog: int - user_event_a: int + user_log: int + user_event_announcements: int verification: int voice_log: int diff --git a/config-default.yml b/config-default.yml index 10cc34816..44da694c2 100644 --- a/config-default.yml +++ b/config-default.yml @@ -110,69 +110,69 @@ guild: id: 267624335836053506 categories: - python_help: 356013061213126657 + python_help: 356013061213126657 channels: - announcements: 354619224620138496 - user_event_a: &USER_EVENT_A 592000283102674944 + announcements: 354619224620138496 + user_event_announcements: &USER_EVENT_A 592000283102674944 # Development - devcontrib: &DEV_CONTRIB 635950537262759947 - devlog: &DEVLOG 622895325144940554 + devcontrib: &DEV_CONTRIB 635950537262759947 + dev_log: &DEVLOG 622895325144940554 # Discussion - meta: 429409067623251969 - python: 267624335836053506 + meta: 429409067623251969 + python_discussion: 267624335836053506 # Logs - attachment_log: &ATTCH_LOG 649243850006855680 - message_log: &MESSAGE_LOG 467752170159079424 - modlog: &MODLOG 282638479504965634 - userlog: 528976905546760203 - voice_log: 640292421988646961 + attachment_log: &ATTACH_LOG 649243850006855680 + message_log: &MESSAGE_LOG 467752170159079424 + mod_log: &MOD_LOG 282638479504965634 + user_log: 528976905546760203 + voice_log: 640292421988646961 # Off-topic - off_topic_0: 291284109232308226 - off_topic_1: 463035241142026251 - off_topic_2: 463035268514185226 + off_topic_0: 291284109232308226 + off_topic_1: 463035241142026251 + off_topic_2: 463035268514185226 # Python Help - help_0: 303906576991780866 - help_1: 303906556754395136 - help_2: 303906514266226689 - help_3: 439702951246692352 - help_4: 451312046647148554 - help_5: 454941769734422538 - help_6: 587375753306570782 - help_7: 587375768556797982 + help_0: 303906576991780866 + help_1: 303906556754395136 + help_2: 303906514266226689 + help_3: 439702951246692352 + help_4: 451312046647148554 + help_5: 454941769734422538 + help_6: 587375753306570782 + help_7: 587375768556797982 # Special - bot: &BOT_CMD 267659945086812160 - esoteric: 470884583684964352 - reddit: 458224812528238616 - verification: 352442727016693763 + bot_commands: &BOT_CMD 267659945086812160 + esoteric: 470884583684964352 + reddit: 458224812528238616 + verification: 352442727016693763 # Staff - admins: &ADMINS 365960823622991872 - admin_spam: &ADMIN_SPAM 563594791770914816 - defcon: &DEFCON 464469101889454091 - helpers: &HELPERS 385474242440986624 - mods: &MODS 305126844661760000 - mod_alerts: 473092532147060736 - mod_spam: &MOD_SPAM 620607373828030464 - organisation: &ORGANISATION 551789653284356126 - staff_lounge: &STAFF_LOUNGE 464905259261755392 + admins: &ADMINS 365960823622991872 + admin_spam: &ADMIN_SPAM 563594791770914816 + defcon: &DEFCON 464469101889454091 + helpers: &HELPERS 385474242440986624 + mods: &MODS 305126844661760000 + mod_alerts: 473092532147060736 + mod_spam: &MOD_SPAM 620607373828030464 + organisation: &ORGANISATION 551789653284356126 + staff_lounge: &STAFF_LOUNGE 464905259261755392 # Voice - admins_voice: &ADMINS_VOICE 500734494840717332 - staff_voice: &STAFF_VOICE 412375055910043655 + admins_voice: &ADMINS_VOICE 500734494840717332 + staff_voice: &STAFF_VOICE 412375055910043655 # Watch - big_brother_logs: &BBLOGS 468507907357409333 - talent_pool: &TALENT_POOL 534321732593647616 + big_brother_logs: &BB_LOGS 468507907357409333 + talent_pool: &TALENT_POOL 534321732593647616 staff_channels: [*ADMINS, *ADMIN_SPAM, *MOD_SPAM, *MODS, *HELPERS, *ORGANISATION, *DEFCON] - ignored: [*ADMINS, *MESSAGE_LOG, *MODLOG, *ADMINS_VOICE, *STAFF_VOICE, *ATTCH_LOG] + ignored: [*ADMINS, *MESSAGE_LOG, *MOD_LOG, *ADMINS_VOICE, *STAFF_VOICE, *ATTACH_LOG] reminder_whitelist: [*BOT_CMD, *DEV_CONTRIB] roles: @@ -193,8 +193,8 @@ guild: owners: &OWNERS_ROLE 267627879762755584 # Code Jam - jammers: 591786436651646989 - team_leaders: 501324292341104650 + jammers: 591786436651646989 + team_leaders: 501324292341104650 webhooks: talent_pool: 569145364800602132 @@ -278,12 +278,11 @@ filter: # Censor doesn't apply to these channel_whitelist: - *ADMINS - - *MODLOG + - *MOD_LOG - *MESSAGE_LOG - - *DEVLOG - - *BBLOGS + - *DEV_LOG + - *BB_LOGS - *STAFF_LOUNGE - - *DEVTEST - *TALENT_POOL - *USER_EVENT_A -- cgit v1.2.3 From b387b2ec3201a33a0f2ba46a032477b126479671 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 12:23:25 -0800 Subject: Always load doc and verification extensions They used to only be loaded in "debug mode" because the main guild was used to test the bot. However, we have since moved to using a separate test guild so it's no longer a concern if these cogs get loaded. It was confusing to some contributors as to why these cogs were not being loaded since the debug mode isn't really documented anywhere. --- bot/__main__.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/bot/__main__.py b/bot/__main__.py index 490163739..0079a9381 100644 --- a/bot/__main__.py +++ b/bot/__main__.py @@ -7,7 +7,7 @@ from sentry_sdk.integrations.logging import LoggingIntegration from bot import patches from bot.bot import Bot -from bot.constants import Bot as BotConfig, DEBUG_MODE +from bot.constants import Bot as BotConfig sentry_logging = LoggingIntegration( level=logging.TRACE, @@ -40,10 +40,8 @@ bot.load_extension("bot.cogs.clean") bot.load_extension("bot.cogs.extensions") bot.load_extension("bot.cogs.help") -# Only load this in production -if not DEBUG_MODE: - bot.load_extension("bot.cogs.doc") - bot.load_extension("bot.cogs.verification") +bot.load_extension("bot.cogs.doc") +bot.load_extension("bot.cogs.verification") # Feature cogs bot.load_extension("bot.cogs.alias") -- cgit v1.2.3 From 92e4e6d250d6de1b164d36b8a16c62dd4c71fa4f Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 12:40:21 -0800 Subject: Config: fix DEV_LOG variable thingy --- config-default.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config-default.yml b/config-default.yml index 44da694c2..51efe4d9a 100644 --- a/config-default.yml +++ b/config-default.yml @@ -117,8 +117,8 @@ guild: user_event_announcements: &USER_EVENT_A 592000283102674944 # Development - devcontrib: &DEV_CONTRIB 635950537262759947 - dev_log: &DEVLOG 622895325144940554 + devcontrib: &DEV_CONTRIB 635950537262759947 + dev_log: &DEV_LOG 622895325144940554 # Discussion meta: 429409067623251969 -- cgit v1.2.3 From 14536f873d5d233880a88c9f71710ef7c2061625 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 12:40:57 -0800 Subject: Config: add underscore to devcontrib --- bot/constants.py | 2 +- config-default.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/constants.py b/bot/constants.py index f35d608da..63f7b15ee 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -364,7 +364,7 @@ class Channels(metaclass=YAMLGetter): big_brother_logs: int bot_commands: int defcon: int - devcontrib: int + dev_contrib: int dev_log: int esoteric: int help_0: int diff --git a/config-default.yml b/config-default.yml index 51efe4d9a..a43610562 100644 --- a/config-default.yml +++ b/config-default.yml @@ -117,7 +117,7 @@ guild: user_event_announcements: &USER_EVENT_A 592000283102674944 # Development - devcontrib: &DEV_CONTRIB 635950537262759947 + dev_contrib: &DEV_CONTRIB 635950537262759947 dev_log: &DEV_LOG 622895325144940554 # Discussion -- cgit v1.2.3 From 7c14787a9b1550328180ac3ce3da4d9faa65f41e Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 13:05:36 -0800 Subject: Config: replace abbreviated lists with normal ones Lists were getting too long to be readable as one line. Having each element on a separate line also reduces merge conflicts. --- config-default.yml | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/config-default.yml b/config-default.yml index a43610562..05059fbee 100644 --- a/config-default.yml +++ b/config-default.yml @@ -171,9 +171,26 @@ guild: big_brother_logs: &BB_LOGS 468507907357409333 talent_pool: &TALENT_POOL 534321732593647616 - staff_channels: [*ADMINS, *ADMIN_SPAM, *MOD_SPAM, *MODS, *HELPERS, *ORGANISATION, *DEFCON] - ignored: [*ADMINS, *MESSAGE_LOG, *MOD_LOG, *ADMINS_VOICE, *STAFF_VOICE, *ATTACH_LOG] - reminder_whitelist: [*BOT_CMD, *DEV_CONTRIB] + staff_channels: + - *ADMINS + - *ADMIN_SPAM + - *DEFCON + - *HELPERS + - *MODS + - *MOD_SPAM + - *ORGANISATION + + ignored: + - *ADMINS + - *ADMINS_VOICE + - *ATTACH_LOG + - *MESSAGE_LOG + - *MOD_LOG + - *STAFF_VOICE + + reminder_whitelist: + - *BOT_CMD + - *DEV_CONTRIB roles: announcements: 463658397560995840 @@ -454,7 +471,20 @@ redirect_output: duck_pond: threshold: 5 - custom_emojis: [*DUCKY_YELLOW, *DUCKY_BLURPLE, *DUCKY_CAMO, *DUCKY_DEVIL, *DUCKY_NINJA, *DUCKY_REGAL, *DUCKY_TUBE, *DUCKY_HUNT, *DUCKY_WIZARD, *DUCKY_PARTY, *DUCKY_ANGEL, *DUCKY_MAUL, *DUCKY_SANTA] + custom_emojis: + - *DUCKY_YELLOW + - *DUCKY_BLURPLE + - *DUCKY_CAMO + - *DUCKY_DEVIL + - *DUCKY_NINJA + - *DUCKY_REGAL + - *DUCKY_TUBE + - *DUCKY_HUNT + - *DUCKY_WIZARD + - *DUCKY_PARTY + - *DUCKY_ANGEL + - *DUCKY_MAUL + - *DUCKY_SANTA config: required_keys: ['bot.token'] -- cgit v1.2.3 From 66fa960fcf0f1d11af20ec1c77039e9ca791f4dc Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 13:08:40 -0800 Subject: Constants: rename Guild.Constant.ignored to modlog_blacklist This name better explains what the list is for. --- bot/cogs/moderation/modlog.py | 10 +++++----- bot/constants.py | 2 +- config-default.yml | 3 ++- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/bot/cogs/moderation/modlog.py b/bot/cogs/moderation/modlog.py index 94e646248..59ae6b587 100644 --- a/bot/cogs/moderation/modlog.py +++ b/bot/cogs/moderation/modlog.py @@ -538,7 +538,7 @@ class ModLog(Cog, name="ModLog"): channel = message.channel author = message.author - if message.guild.id != GuildConstant.id or channel.id in GuildConstant.ignored: + if message.guild.id != GuildConstant.id or channel.id in GuildConstant.modlog_blacklist: return self._cached_deletes.append(message.id) @@ -591,7 +591,7 @@ class ModLog(Cog, name="ModLog"): @Cog.listener() async def on_raw_message_delete(self, event: discord.RawMessageDeleteEvent) -> None: """Log raw message delete event to message change log.""" - if event.guild_id != GuildConstant.id or event.channel_id in GuildConstant.ignored: + if event.guild_id != GuildConstant.id or event.channel_id in GuildConstant.modlog_blacklist: return await asyncio.sleep(1) # Wait here in case the normal event was fired @@ -635,7 +635,7 @@ class ModLog(Cog, name="ModLog"): if ( not msg_before.guild or msg_before.guild.id != GuildConstant.id - or msg_before.channel.id in GuildConstant.ignored + or msg_before.channel.id in GuildConstant.modlog_blacklist or msg_before.author.bot ): return @@ -717,7 +717,7 @@ class ModLog(Cog, name="ModLog"): if ( not message.guild or message.guild.id != GuildConstant.id - or message.channel.id in GuildConstant.ignored + or message.channel.id in GuildConstant.modlog_blacklist or message.author.bot ): return @@ -769,7 +769,7 @@ class ModLog(Cog, name="ModLog"): """Log member voice state changes to the voice log channel.""" if ( member.guild.id != GuildConstant.id - or (before.channel and before.channel.id in GuildConstant.ignored) + or (before.channel and before.channel.id in GuildConstant.modlog_blacklist) ): return diff --git a/bot/constants.py b/bot/constants.py index 63f7b15ee..9855421c9 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -428,7 +428,7 @@ class Guild(metaclass=YAMLGetter): section = "guild" id: int - ignored: List[int] + modlog_blacklist: List[int] staff_channels: List[int] reminder_whitelist: List[int] diff --git a/config-default.yml b/config-default.yml index 05059fbee..b253f32e8 100644 --- a/config-default.yml +++ b/config-default.yml @@ -180,7 +180,8 @@ guild: - *MOD_SPAM - *ORGANISATION - ignored: + # Modlog cog ignores events which occur in these channels + modlog_blacklist: - *ADMINS - *ADMINS_VOICE - *ATTACH_LOG -- cgit v1.2.3 From 12c7d2794e8e9d086f5d52c8916df26bb9de5979 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Sun, 23 Feb 2020 13:29:08 -0800 Subject: Tests: fix setting bot-commands ID in information tests --- tests/bot/cogs/test_information.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/bot/cogs/test_information.py b/tests/bot/cogs/test_information.py index 38293269f..8443cfe71 100644 --- a/tests/bot/cogs/test_information.py +++ b/tests/bot/cogs/test_information.py @@ -521,7 +521,7 @@ class UserCommandTests(unittest.TestCase): """A regular user should not be able to use this command outside of bot-commands.""" constants.MODERATION_ROLES = [self.moderator_role.id] constants.STAFF_ROLES = [self.moderator_role.id] - constants.Channels.bot = 50 + constants.Channels.bot_commands = 50 ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=100)) @@ -533,7 +533,7 @@ class UserCommandTests(unittest.TestCase): def test_regular_user_may_use_command_in_bot_commands_channel(self, create_embed, constants): """A regular user should be allowed to use `!user` targeting themselves in bot-commands.""" constants.STAFF_ROLES = [self.moderator_role.id] - constants.Channels.bot = 50 + constants.Channels.bot_commands = 50 ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=50)) @@ -546,7 +546,7 @@ class UserCommandTests(unittest.TestCase): def test_regular_user_can_explicitly_target_themselves(self, create_embed, constants): """A user should target itself with `!user` when a `user` argument was not provided.""" constants.STAFF_ROLES = [self.moderator_role.id] - constants.Channels.bot = 50 + constants.Channels.bot_commands = 50 ctx = helpers.MockContext(author=self.author, channel=helpers.MockTextChannel(id=50)) @@ -559,7 +559,7 @@ class UserCommandTests(unittest.TestCase): def test_staff_members_can_bypass_channel_restriction(self, create_embed, constants): """Staff members should be able to bypass the bot-commands channel restriction.""" constants.STAFF_ROLES = [self.moderator_role.id] - constants.Channels.bot = 50 + constants.Channels.bot_commands = 50 ctx = helpers.MockContext(author=self.moderator, channel=helpers.MockTextChannel(id=200)) -- cgit v1.2.3 From 8a3063be1764307d05ae0215b00f53b06ed33f6c Mon Sep 17 00:00:00 2001 From: Numerlor Date: Mon, 24 Feb 2020 01:30:50 +0100 Subject: Implement `__iter__` on constants YAMLGetter. Python tries to fall back on passing indices to `__getitem__` without iter implemented; failing on the first line. --- bot/constants.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/bot/constants.py b/bot/constants.py index a4c65a1f8..3ecdb5b35 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -186,6 +186,10 @@ class YAMLGetter(type): def __getitem__(cls, name): return cls.__getattr__(name) + def __iter__(cls): + """Returns iterator of key: value pairs of current constants class.""" + return iter(_CONFIG_YAML[cls.section][cls.subsection].items()) + # Dataclasses class Bot(metaclass=YAMLGetter): -- cgit v1.2.3 From 08f6ed038fa4be5ed09227114902a52d72a73155 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Mon, 24 Feb 2020 01:33:23 +0100 Subject: Add ConfigVerifier cog. Adds ConfigVerifier which verifies channels when loaded. --- bot/__main__.py | 1 + bot/cogs/config_verifier.py | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) create mode 100644 bot/cogs/config_verifier.py diff --git a/bot/__main__.py b/bot/__main__.py index 490163739..79f89b467 100644 --- a/bot/__main__.py +++ b/bot/__main__.py @@ -31,6 +31,7 @@ bot.load_extension("bot.cogs.error_handler") bot.load_extension("bot.cogs.filtering") bot.load_extension("bot.cogs.logging") bot.load_extension("bot.cogs.security") +bot.load_extension("bot.cogs.config_verifier") # Commands, etc bot.load_extension("bot.cogs.antimalware") diff --git a/bot/cogs/config_verifier.py b/bot/cogs/config_verifier.py new file mode 100644 index 000000000..f0aaa06ea --- /dev/null +++ b/bot/cogs/config_verifier.py @@ -0,0 +1,40 @@ +import logging + +from discord.ext.commands import Cog + +from bot import constants +from bot.bot import Bot + + +log = logging.getLogger(__name__) + + +class ConfigVerifier(Cog): + """Verify config on startup.""" + + def __init__(self, bot: Bot): + self.bot = bot + self.bot.loop.create_task(self.verify_channels()) + + async def verify_channels(self) -> None: + """ + Verifies channels in config. + + If any channels in config aren't present in server, log them in a warning. + """ + await self.bot.wait_until_ready() + server = self.bot.get_guild(constants.Guild.id) + + server_channel_ids = {channel.id for channel in server.channels} + invalid_channels = [ + channel_name for channel_name, channel_id in constants.Channels + if channel_id not in server_channel_ids + ] + + if invalid_channels: + log.warning(f"Channels do not exist in server: {', '.join(invalid_channels)}.") + + +def setup(bot: Bot) -> None: + """Load the ConfigVerifier cog.""" + bot.add_cog(ConfigVerifier(bot)) -- cgit v1.2.3 From a1ad4ae66bfa14972f9ea686a728e8060bfe55e0 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Mon, 24 Feb 2020 01:43:43 +0100 Subject: Change warning text. --- bot/cogs/config_verifier.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/config_verifier.py b/bot/cogs/config_verifier.py index f0aaa06ea..d2bc81ba6 100644 --- a/bot/cogs/config_verifier.py +++ b/bot/cogs/config_verifier.py @@ -32,7 +32,7 @@ class ConfigVerifier(Cog): ] if invalid_channels: - log.warning(f"Channels do not exist in server: {', '.join(invalid_channels)}.") + log.warning(f"Configured channels do not exist in server: {', '.join(invalid_channels)}.") def setup(bot: Bot) -> None: -- cgit v1.2.3 From 62a232b3a55a7cc983487ac165a4a9bbd6d6e3f9 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Mon, 24 Feb 2020 16:56:35 +0100 Subject: Change docstring mood. --- bot/cogs/config_verifier.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/config_verifier.py b/bot/cogs/config_verifier.py index d2bc81ba6..cc19f7423 100644 --- a/bot/cogs/config_verifier.py +++ b/bot/cogs/config_verifier.py @@ -18,7 +18,7 @@ class ConfigVerifier(Cog): async def verify_channels(self) -> None: """ - Verifies channels in config. + Verify channels. If any channels in config aren't present in server, log them in a warning. """ -- cgit v1.2.3 From 6174792c01f238e32aca5cc9222caa4feb788281 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Mon, 24 Feb 2020 19:45:09 +0100 Subject: Remove unused `chunks` function and its tests. The function was only used in the since removed `Events` cog. --- bot/utils/__init__.py | 12 +----------- tests/bot/test_utils.py | 15 --------------- 2 files changed, 1 insertion(+), 26 deletions(-) diff --git a/bot/utils/__init__.py b/bot/utils/__init__.py index 8184be824..3e4b15ce4 100644 --- a/bot/utils/__init__.py +++ b/bot/utils/__init__.py @@ -1,5 +1,5 @@ from abc import ABCMeta -from typing import Any, Generator, Hashable, Iterable +from typing import Any, Hashable from discord.ext.commands import CogMeta @@ -64,13 +64,3 @@ class CaseInsensitiveDict(dict): for k in list(self.keys()): v = super(CaseInsensitiveDict, self).pop(k) self.__setitem__(k, v) - - -def chunks(iterable: Iterable, size: int) -> Generator[Any, None, None]: - """ - Generator that allows you to iterate over any indexable collection in `size`-length chunks. - - Found: https://stackoverflow.com/a/312464/4022104 - """ - for i in range(0, len(iterable), size): - yield iterable[i:i + size] diff --git a/tests/bot/test_utils.py b/tests/bot/test_utils.py index 58ae2a81a..d7bcc3ba6 100644 --- a/tests/bot/test_utils.py +++ b/tests/bot/test_utils.py @@ -35,18 +35,3 @@ class CaseInsensitiveDictTests(unittest.TestCase): instance = utils.CaseInsensitiveDict() instance.update({'FOO': 'bar'}) self.assertEqual(instance['foo'], 'bar') - - -class ChunkTests(unittest.TestCase): - """Tests the `chunk` method.""" - - def test_empty_chunking(self): - """Tests chunking on an empty iterable.""" - generator = utils.chunks(iterable=[], size=5) - self.assertEqual(list(generator), []) - - def test_list_chunking(self): - """Tests chunking a non-empty list.""" - iterable = [1, 2, 3, 4, 5] - generator = utils.chunks(iterable=iterable, size=2) - self.assertEqual(list(generator), [[1, 2], [3, 4], [5]]) -- cgit v1.2.3 From b4ed7107d162d1961ae4dc03cdda282123fbb877 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Mon, 24 Feb 2020 22:22:11 +0100 Subject: Do not attempt to load Reddit cog when environment variables are not provided. When environment variables weren't provided; the cog attempted to create a BasicAuth object with None as values resulting in an exception before the event loop was started and a subsequent crash. --- bot/cogs/reddit.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index aa487f18e..dce73fcf2 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -290,4 +290,7 @@ class Reddit(Cog): def setup(bot: Bot) -> None: """Load the Reddit cog.""" - bot.add_cog(Reddit(bot)) + if None not in (RedditConfig.client_id, RedditConfig.secret): + bot.add_cog(Reddit(bot)) + return + log.error("Credentials not provided, cog not loaded.") -- cgit v1.2.3 From daf50941ca6ceaa0b65d71cd9fee1ad2a67e1718 Mon Sep 17 00:00:00 2001 From: Numerlor Date: Tue, 25 Feb 2020 14:19:00 +0100 Subject: Wait for available guild instead of bot startup. Co-authored-by: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> --- bot/cogs/config_verifier.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/config_verifier.py b/bot/cogs/config_verifier.py index cc19f7423..b43b48264 100644 --- a/bot/cogs/config_verifier.py +++ b/bot/cogs/config_verifier.py @@ -22,7 +22,7 @@ class ConfigVerifier(Cog): If any channels in config aren't present in server, log them in a warning. """ - await self.bot.wait_until_ready() + await self.bot.wait_until_guild_available() server = self.bot.get_guild(constants.Guild.id) server_channel_ids = {channel.id for channel in server.channels} -- cgit v1.2.3 From d6ef05c28021db5087ce1a27a108c35c276b915f Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 25 Feb 2020 14:32:32 +0100 Subject: Assign created task to a variable. Co-authored-by: SebastiaanZ <33516116+SebastiaanZ@users.noreply.github.com> --- bot/cogs/config_verifier.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/config_verifier.py b/bot/cogs/config_verifier.py index b43b48264..d72c6c22e 100644 --- a/bot/cogs/config_verifier.py +++ b/bot/cogs/config_verifier.py @@ -14,7 +14,7 @@ class ConfigVerifier(Cog): def __init__(self, bot: Bot): self.bot = bot - self.bot.loop.create_task(self.verify_channels()) + self.channel_verify_task = self.bot.loop.create_task(self.verify_channels()) async def verify_channels(self) -> None: """ -- cgit v1.2.3 From 3b3206471c028f87685c4c07db0c167a7066ced2 Mon Sep 17 00:00:00 2001 From: "S. Co1" Date: Tue, 25 Feb 2020 12:28:10 -0500 Subject: Configure staff role & channel groupings in YAML Delete duplicate keys that were missed in the merge --- bot/constants.py | 11 +++++++---- config-default.yml | 26 ++++++++++++++++++++------ 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/bot/constants.py b/bot/constants.py index 285761055..b1713aa60 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -431,9 +431,12 @@ class Guild(metaclass=YAMLGetter): section = "guild" id: int + moderation_channels: List[int] + moderation_roles: List[int] modlog_blacklist: List[int] - staff_channels: List[int] reminder_whitelist: List[int] + staff_channels: List[int] + staff_roles: List[int] class Keys(metaclass=YAMLGetter): section = "keys" @@ -579,14 +582,14 @@ BOT_DIR = os.path.dirname(__file__) PROJECT_ROOT = os.path.abspath(os.path.join(BOT_DIR, os.pardir)) # Default role combinations -MODERATION_ROLES = Roles.moderators, Roles.admins, Roles.owners -STAFF_ROLES = Roles.helpers, Roles.moderators, Roles.admins, Roles.owners +MODERATION_ROLES = Guild.moderation_roles +STAFF_ROLES = Guild.staff_roles # Roles combinations STAFF_CHANNELS = Guild.staff_channels # Default Channel combinations -MODERATION_CHANNELS = Channels.admins, Channels.admin_spam, Channels.mod_alerts, Channels.mods, Channels.mod_spam +MODERATION_CHANNELS = Guild.moderation_channels # Bot replies diff --git a/config-default.yml b/config-default.yml index dca00500e..ab237423f 100644 --- a/config-default.yml +++ b/config-default.yml @@ -160,7 +160,7 @@ guild: defcon: &DEFCON 464469101889454091 helpers: &HELPERS 385474242440986624 mods: &MODS 305126844661760000 - mod_alerts: 473092532147060736 + mod_alerts: &MOD_ALERTS 473092532147060736 mod_spam: &MOD_SPAM 620607373828030464 organisation: &ORGANISATION 551789653284356126 staff_lounge: &STAFF_LOUNGE 464905259261755392 @@ -182,6 +182,13 @@ guild: - *MOD_SPAM - *ORGANISATION + moderation_channels: + - *ADMINS + - *ADMIN_SPAM + - *MOD_ALERTS + - *MODS + - *MOD_SPAM + # Modlog cog ignores events which occur in these channels modlog_blacklist: - *ADMINS @@ -195,10 +202,6 @@ guild: - *BOT_CMD - *DEV_CONTRIB - staff_channels: [*ADMINS, *ADMIN_SPAM, *MOD_SPAM, *MODS, *HELPERS, *ORGANISATION, *DEFCON] - ignored: [*ADMINS, *MESSAGE_LOG, *MODLOG, *ADMINS_VOICE, *STAFF_VOICE, *ATTCH_LOG] - reminder_whitelist: [*BOT_CMD, *DEV_CONTRIB] - roles: announcements: 463658397560995840 contributors: 295488872404484098 @@ -212,7 +215,7 @@ guild: # Staff admins: &ADMINS_ROLE 267628507062992896 core_developers: 587606783669829632 - helpers: 267630620367257601 + helpers: &HELPERS_ROLE 267630620367257601 moderators: &MODS_ROLE 267629731250176001 owners: &OWNERS_ROLE 267627879762755584 @@ -220,6 +223,17 @@ guild: jammers: 591786436651646989 team_leaders: 501324292341104650 + moderation_roles: + - *OWNERS_ROLE + - *ADMINS_ROLE + - *MODS_ROLE + + staff_roles: + - *OWNERS_ROLE + - *ADMINS_ROLE + - *MODS_ROLE + - *HELPERS_ROLE + webhooks: talent_pool: 569145364800602132 big_brother: 569133704568373283 -- cgit v1.2.3 From 284c1de321fea5927dafc1ac3192ad763bda3203 Mon Sep 17 00:00:00 2001 From: "S. Co1" Date: Tue, 25 Feb 2020 12:47:09 -0500 Subject: Fix mismatched constant names in syncer tests --- bot/cogs/sync/syncers.py | 8 ++++---- tests/bot/cogs/sync/test_base.py | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/bot/cogs/sync/syncers.py b/bot/cogs/sync/syncers.py index 6715ad6fb..d6891168f 100644 --- a/bot/cogs/sync/syncers.py +++ b/bot/cogs/sync/syncers.py @@ -23,7 +23,7 @@ _Diff = namedtuple('Diff', ('created', 'updated', 'deleted')) class Syncer(abc.ABC): """Base class for synchronising the database with objects in the Discord cache.""" - _CORE_DEV_MENTION = f"<@&{constants.Roles.core_developer}> " + _CORE_DEV_MENTION = f"<@&{constants.Roles.core_developers}> " _REACTION_EMOJIS = (constants.Emojis.check_mark, constants.Emojis.cross_mark) def __init__(self, bot: Bot) -> None: @@ -54,12 +54,12 @@ class Syncer(abc.ABC): # Send to core developers if it's an automatic sync. if not message: log.trace("Message not provided for confirmation; creating a new one in dev-core.") - channel = self.bot.get_channel(constants.Channels.devcore) + channel = self.bot.get_channel(constants.Channels.dev_core) if not channel: log.debug("Failed to get the dev-core channel from cache; attempting to fetch it.") try: - channel = await self.bot.fetch_channel(constants.Channels.devcore) + channel = await self.bot.fetch_channel(constants.Channels.dev_core) except HTTPException: log.exception( f"Failed to fetch channel for sending sync confirmation prompt; " @@ -93,7 +93,7 @@ class Syncer(abc.ABC): `author` of the prompt. """ # For automatic syncs, check for the core dev role instead of an exact author - has_role = any(constants.Roles.core_developer == role.id for role in user.roles) + has_role = any(constants.Roles.core_developers == role.id for role in user.roles) return ( reaction.message.id == message.id and not user.bot diff --git a/tests/bot/cogs/sync/test_base.py b/tests/bot/cogs/sync/test_base.py index e6a6f9688..c2e143865 100644 --- a/tests/bot/cogs/sync/test_base.py +++ b/tests/bot/cogs/sync/test_base.py @@ -84,7 +84,7 @@ class SyncerSendPromptTests(unittest.TestCase): mock_() await self.syncer._send_prompt() - method.assert_called_once_with(constants.Channels.devcore) + method.assert_called_once_with(constants.Channels.dev_core) @helpers.async_test async def test_send_prompt_returns_None_if_channel_fetch_fails(self): @@ -135,7 +135,7 @@ class SyncerConfirmationTests(unittest.TestCase): def setUp(self): self.bot = helpers.MockBot() self.syncer = TestSyncer(self.bot) - self.core_dev_role = helpers.MockRole(id=constants.Roles.core_developer) + self.core_dev_role = helpers.MockRole(id=constants.Roles.core_developers) @staticmethod def get_message_reaction(emoji): -- cgit v1.2.3 From 33302afd9e83cb4b5502a8b5bbe43bac450dba3f Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 25 Feb 2020 22:54:03 +0100 Subject: Fix `__iter__` for classes without subsections. The previous implementation assumed the config class was a subsection, failing with a KeyError if it wasn't one. Co-authored-by: kwzrd --- bot/constants.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bot/constants.py b/bot/constants.py index 3776ceb84..ebd3b3d96 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -187,8 +187,9 @@ class YAMLGetter(type): return cls.__getattr__(name) def __iter__(cls): - """Returns iterator of key: value pairs of current constants class.""" - return iter(_CONFIG_YAML[cls.section][cls.subsection].items()) + """Return generator of key: value pairs of current constants class' config values.""" + for name in cls.__annotations__: + yield name, getattr(cls, name) # Dataclasses -- cgit v1.2.3 From 1f82ed36f24f2ffbef5b3601fb6c11db28735c71 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Tue, 25 Feb 2020 22:57:52 +0100 Subject: Restyle if body to include the error instead of adding the cog. --- bot/cogs/reddit.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index e93e4de0c..3278363ba 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -290,7 +290,7 @@ class Reddit(Cog): def setup(bot: Bot) -> None: """Load the Reddit cog.""" - if None not in (RedditConfig.client_id, RedditConfig.secret): - bot.add_cog(Reddit(bot)) + if None in (RedditConfig.client_id, RedditConfig.secret): + log.error("Credentials not provided, cog not loaded.") return - log.error("Credentials not provided, cog not loaded.") + bot.add_cog(Reddit(bot)) -- cgit v1.2.3 From 6a2a2b5c3da79fe0097c98a04e435e493c73223d Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 00:29:52 +0100 Subject: Check for empty strings alongside None before loading cog. Docker fetches values from the .env itself and defaults to "" instead of None, needing to do invalid access token requests before unloading itself. --- bot/cogs/reddit.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index 3278363ba..7cb340145 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -290,7 +290,8 @@ class Reddit(Cog): def setup(bot: Bot) -> None: """Load the Reddit cog.""" - if None in (RedditConfig.client_id, RedditConfig.secret): + invalid_values = "", None + if any(value in (RedditConfig.secret, RedditConfig.client_id) for value in invalid_values): log.error("Credentials not provided, cog not loaded.") return bot.add_cog(Reddit(bot)) -- cgit v1.2.3 From e8e2fa9ee8f607bb6593b7c8325446dc074a972d Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 00:33:29 +0100 Subject: Make sure token exists before checking its expiration. Without the check and an invalid token, an AttributeError is raised; blocking the relevant ClientError from being raised in `get_access_token`. --- bot/cogs/reddit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index 7cb340145..982c0cbe6 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -43,7 +43,7 @@ class Reddit(Cog): def cog_unload(self) -> None: """Stop the loop task and revoke the access token when the cog is unloaded.""" self.auto_poster_loop.cancel() - if self.access_token.expires_at < datetime.utcnow(): + if self.access_token and self.access_token.expires_at < datetime.utcnow(): self.revoke_access_token() async def init_reddit_ready(self) -> None: -- cgit v1.2.3 From df87aba432db50eb480ba8b2f42b1a64147909d9 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Tue, 25 Feb 2020 20:15:58 -0800 Subject: Moderation: use asyncio.shield to prevent self-cancellation The shield exists to be used for exactly this purpose so its a better fit than create_task. --- bot/cogs/moderation/scheduler.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bot/cogs/moderation/scheduler.py b/bot/cogs/moderation/scheduler.py index 162159af8..93afd9f9f 100644 --- a/bot/cogs/moderation/scheduler.py +++ b/bot/cogs/moderation/scheduler.py @@ -1,3 +1,4 @@ +import asyncio import logging import textwrap import typing as t @@ -427,6 +428,6 @@ class InfractionScheduler(Scheduler): expiry = dateutil.parser.isoparse(infraction["expires_at"]).replace(tzinfo=None) await time.wait_until(expiry) - # Because deactivate_infraction() explicitly cancels this scheduled task, it runs in - # a separate task to avoid prematurely cancelling itself. - self.bot.loop.create_task(self.deactivate_infraction(infraction)) + # Because deactivate_infraction() explicitly cancels this scheduled task, it is shielded + # to avoid prematurely cancelling itself. + await asyncio.shield(self.deactivate_infraction(infraction)) -- cgit v1.2.3 From b1f8f4779738be35e1339d6c07e317ef08009467 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Tue, 25 Feb 2020 21:20:20 -0800 Subject: Scheduler: improve cancel_task's docstring * Use imperative mood for docstring * Explain the purpose of the parameter in the docstring * Make log message after cog name lowercase --- bot/utils/scheduling.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py index 0d66952eb..cb28648db 100644 --- a/bot/utils/scheduling.py +++ b/bot/utils/scheduling.py @@ -52,19 +52,19 @@ class Scheduler(metaclass=CogABCMeta): log.debug(f"{self.cog_name}: scheduled task #{task_id} {id(task)}.") def cancel_task(self, task_id: str) -> None: - """Un-schedules a task.""" + """Unschedule the task identified by `task_id`.""" log.trace(f"{self.cog_name}: cancelling task #{task_id}...") - task = self._scheduled_tasks.get(task_id) - if task is None: - log.warning(f"{self.cog_name}: Failed to unschedule {task_id} (no task found).") + if not task: + log.warning(f"{self.cog_name}: failed to unschedule {task_id} (no task found).") return task.cancel() - log.debug(f"{self.cog_name}: unscheduled task #{task_id} {id(task)}.") del self._scheduled_tasks[task_id] + log.debug(f"{self.cog_name}: unscheduled task #{task_id} {id(task)}.") + def _task_done_callback(self, task_id: str, task: asyncio.Task) -> None: """ Unschedule the task and raise its exception if one exists. -- cgit v1.2.3 From 4a11bf22cc9e894271b896eb9fca0c3cff085766 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Tue, 25 Feb 2020 22:22:01 -0800 Subject: Scheduler: only delete the task in the done callback if tasks are same To prevent a deletion of task rescheduled with the same ID, the callback checks that the stored task is the same as the done task being handled. * Only delete the task; it doesn't need to be cancelled because the it is already done * Revise the callback's docstring to explain the new behaviour * Rename `task` parameter to `done_task` --- bot/utils/scheduling.py | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py index cb28648db..58bb32e5d 100644 --- a/bot/utils/scheduling.py +++ b/bot/utils/scheduling.py @@ -65,24 +65,33 @@ class Scheduler(metaclass=CogABCMeta): log.debug(f"{self.cog_name}: unscheduled task #{task_id} {id(task)}.") - def _task_done_callback(self, task_id: str, task: asyncio.Task) -> None: + def _task_done_callback(self, task_id: str, done_task: asyncio.Task) -> None: """ - Unschedule the task and raise its exception if one exists. + Delete the task and raise its exception if one exists. - If the task was cancelled, the CancelledError is retrieved and suppressed. In this case, - the task is already assumed to have been unscheduled. + If `done_task` and the task associated with `task_id` are different, then the latter + will not be deleted. In this case, a new task was likely rescheduled with the same ID. """ - log.trace(f"{self.cog_name}: performing done callback for task #{task_id} {id(task)}") + log.trace(f"{self.cog_name}: performing done callback for task #{task_id} {id(done_task)}.") - if task.cancelled(): - with contextlib.suppress(asyncio.CancelledError): - task.exception() + scheduled_task = self._scheduled_tasks.get(task_id) + + if scheduled_task and done_task is scheduled_task: + # A task for the ID exists and its the same as the done task. + # Since this is the done callback, the task is already done so no need to cancel it. + log.trace(f"{self.cog_name}: deleting task #{task_id} {id(done_task)}.") + del self._scheduled_tasks[task_id] + elif scheduled_task: + # A new task was likely rescheduled with the same ID. + log.debug( + f"{self.cog_name}: the scheduled task #{task_id} {id(scheduled_task)} " + f"and the done task {id(done_task)} differ." + ) else: - # Check if it exists to avoid logging a warning. - if task_id in self._scheduled_tasks: - # Only cancel if the task is not cancelled to avoid a race condition when a new - # task is scheduled using the same ID. Reminders do this when re-scheduling after - # editing. - self.cancel_task(task_id) - - task.exception() + log.warning( + f"{self.cog_name}: task #{task_id} not found while handling task {id(done_task)}! " + f"A task somehow got unscheduled improperly (i.e. deleted but not cancelled)." + ) + + with contextlib.suppress(asyncio.CancelledError): + done_task.exception() -- cgit v1.2.3 From 47b645a2cd2622709c57158d788554544579d870 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Tue, 25 Feb 2020 22:23:07 -0800 Subject: Scheduler: properly raise task's exception the done callback Task.exception() only returns the exception. It still needs to be explicitly raised. --- bot/utils/scheduling.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py index 58bb32e5d..742133f02 100644 --- a/bot/utils/scheduling.py +++ b/bot/utils/scheduling.py @@ -94,4 +94,7 @@ class Scheduler(metaclass=CogABCMeta): ) with contextlib.suppress(asyncio.CancelledError): - done_task.exception() + exception = done_task.exception() + # Raise the exception if one exists. + if exception: + raise exception -- cgit v1.2.3 From e173cd2af20b546230ed467f26286ee167df55cd Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Tue, 25 Feb 2020 22:34:35 -0800 Subject: Scheduler: only send warning in callback if task isn't cancelled If a task is cancelled it is assumed it was done via cancel_task. That method deletes the task after cancelling so the warning isn't relevant. --- bot/utils/scheduling.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py index 742133f02..9371dcdb7 100644 --- a/bot/utils/scheduling.py +++ b/bot/utils/scheduling.py @@ -87,7 +87,7 @@ class Scheduler(metaclass=CogABCMeta): f"{self.cog_name}: the scheduled task #{task_id} {id(scheduled_task)} " f"and the done task {id(done_task)} differ." ) - else: + elif not done_task.cancelled(): log.warning( f"{self.cog_name}: task #{task_id} not found while handling task {id(done_task)}! " f"A task somehow got unscheduled improperly (i.e. deleted but not cancelled)." -- cgit v1.2.3 From d91f821fccfa61f324489baff5debbebb3adbb51 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 08:42:30 +0100 Subject: Create task for `revoke_access_token` when unloading cog to ensure it's executed. --- bot/cogs/reddit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index 982c0cbe6..6fe7f820b 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -44,7 +44,7 @@ class Reddit(Cog): """Stop the loop task and revoke the access token when the cog is unloaded.""" self.auto_poster_loop.cancel() if self.access_token and self.access_token.expires_at < datetime.utcnow(): - self.revoke_access_token() + asyncio.create_task(self.revoke_access_token()) async def init_reddit_ready(self) -> None: """Sets the reddit webhook when the cog is loaded.""" -- cgit v1.2.3 From 5a0a04ea4fb4a9aa17e7f807e72f2bcd5e3e6349 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 08:43:06 +0100 Subject: Specify the logged time is in UTC. --- bot/cogs/reddit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index 6fe7f820b..22e5c2a9c 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -83,7 +83,7 @@ class Reddit(Cog): expires_at=datetime.utcnow() + timedelta(seconds=expiration) ) - log.debug(f"New token acquired; expires on {self.access_token.expires_at}") + log.debug(f"New token acquired; expires on UTC {self.access_token.expires_at}") return else: log.debug( -- cgit v1.2.3 From f87a53ff442bc10cd2cba87943e40c531e0ce9ba Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 10:43:38 +0100 Subject: Check for falsy values instead of ``""` and `None` explicitly. After the change to also check empty strings to avoid unucessary requests, it is no longer necessary to do an explicit value check, as the only values that can come from the .env file are `None` and strings Co-authored-by: Karlis S <45097959+ks129@users.noreply.github.com> --- bot/cogs/reddit.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index 22e5c2a9c..6d03928e0 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -290,8 +290,7 @@ class Reddit(Cog): def setup(bot: Bot) -> None: """Load the Reddit cog.""" - invalid_values = "", None - if any(value in (RedditConfig.secret, RedditConfig.client_id) for value in invalid_values): + if not RedditConfig.secret or not RedditConfig.client_id: log.error("Credentials not provided, cog not loaded.") return bot.add_cog(Reddit(bot)) -- cgit v1.2.3 From c2d695fe196c49d3dfcae1186c1dafe13ba98e88 Mon Sep 17 00:00:00 2001 From: "Karlis. S" Date: Tue, 25 Feb 2020 20:01:15 +0200 Subject: Added to AntiMalware staff ignore check. --- bot/cogs/antimalware.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bot/cogs/antimalware.py b/bot/cogs/antimalware.py index 28e3e5d96..d2ff9f79c 100644 --- a/bot/cogs/antimalware.py +++ b/bot/cogs/antimalware.py @@ -4,7 +4,7 @@ from discord import Embed, Message, NotFound from discord.ext.commands import Cog from bot.bot import Bot -from bot.constants import AntiMalware as AntiMalwareConfig, Channels, URLs +from bot.constants import AntiMalware as AntiMalwareConfig, Channels, STAFF_ROLES, URLs log = logging.getLogger(__name__) @@ -21,6 +21,10 @@ class AntiMalware(Cog): if not message.attachments: return + # Check if user is staff, if is, return + if hasattr(message.author, "roles") and any(role.id in STAFF_ROLES for role in message.author.roles): + return + embed = Embed() for attachment in message.attachments: filename = attachment.filename.lower() -- cgit v1.2.3 From f779f60b5376872043eda8c25712f0dd2a451a78 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 18:20:19 +0100 Subject: Fix comparison operator when checking token expiration. With `<` the check only went through when the token was already expired, making revoking redundant; and didn't go through when the token still had some time before expiration. --- bot/cogs/reddit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/cogs/reddit.py b/bot/cogs/reddit.py index 6d03928e0..5a7fa100f 100644 --- a/bot/cogs/reddit.py +++ b/bot/cogs/reddit.py @@ -43,7 +43,7 @@ class Reddit(Cog): def cog_unload(self) -> None: """Stop the loop task and revoke the access token when the cog is unloaded.""" self.auto_poster_loop.cancel() - if self.access_token and self.access_token.expires_at < datetime.utcnow(): + if self.access_token and self.access_token.expires_at > datetime.utcnow(): asyncio.create_task(self.revoke_access_token()) async def init_reddit_ready(self) -> None: -- cgit v1.2.3 From b628c5b9a054af22851aa099f0656ed465472456 Mon Sep 17 00:00:00 2001 From: ks123 Date: Wed, 26 Feb 2020 19:34:01 +0200 Subject: Added DMs ignoring to antimalware check --- bot/cogs/antimalware.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bot/cogs/antimalware.py b/bot/cogs/antimalware.py index d2ff9f79c..957c458c0 100644 --- a/bot/cogs/antimalware.py +++ b/bot/cogs/antimalware.py @@ -18,7 +18,8 @@ class AntiMalware(Cog): @Cog.listener() async def on_message(self, message: Message) -> None: """Identify messages with prohibited attachments.""" - if not message.attachments: + # Return when message don't have attachment and don't moderate DMs + if not message.attachments or not message.guild: return # Check if user is staff, if is, return -- cgit v1.2.3 From 68f97584d0e472857f07f8421001e007d5983164 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 19:00:52 +0100 Subject: Make sure tag name contains at least one letter. With only ascii and numbers being allowed to go through, possible values still included things like `$()` which don't match anything in `REGEX_NON_ALPHABET` from tags.py resulting in an error. --- bot/converters.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bot/converters.py b/bot/converters.py index cca57a02d..d73ab73f1 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -175,6 +175,12 @@ class TagNameConverter(Converter): "Rejecting the request.") raise BadArgument("Are you insane? That's way too long!") + # The tag name is ascii but does not contain any letters. + elif not any(character.isalpha() for character in tag_name): + log.warning(f"{ctx.author} tried to request a tag name without letters. " + "Rejecting the request.") + raise BadArgument("Tag names must contain at least one letter.") + return tag_name -- cgit v1.2.3 From b4a52aded317572d51a0747ed8d74b3fc84c9428 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 19:13:48 +0100 Subject: Pass error handler tag fallback through TagNameConverter. The tag fallback didn't convert tags, resulting in possible invalid tag names being passed to the `tags_get_command`. This makes sure they're valid and ignores the risen exception if they are not. --- bot/cogs/error_handler.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 0abb7e521..3486a746c 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -20,6 +20,7 @@ from sentry_sdk import push_scope from bot.api import ResponseCodeError from bot.bot import Bot from bot.constants import Channels +from bot.converters import TagNameConverter from bot.decorators import InChannelCheckFailure log = logging.getLogger(__name__) @@ -88,8 +89,11 @@ class ErrorHandler(Cog): return # Return to not raise the exception - with contextlib.suppress(ResponseCodeError): - await ctx.invoke(tags_get_command, tag_name=ctx.invoked_with) + with contextlib.suppress(BadArgument, ResponseCodeError): + await ctx.invoke( + tags_get_command, + tag_name=await TagNameConverter.convert(ctx, ctx.invoked_with) + ) return elif isinstance(e, BadArgument): await ctx.send(f"Bad argument: {e}\n") -- cgit v1.2.3 From 432311ce720a1aea23c3ed7b422615a2e304b070 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 19:22:28 +0100 Subject: Remove number check on tags. This case is already covered by checking if at least one letter is included. --- bot/converters.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index d73ab73f1..745ce5b5d 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -141,14 +141,6 @@ class TagNameConverter(Converter): @staticmethod async def convert(ctx: Context, tag_name: str) -> str: """Lowercase & strip whitespace from proposed tag_name & ensure it's valid.""" - def is_number(value: str) -> bool: - """Check to see if the input string is numeric.""" - try: - float(value) - except ValueError: - return False - return True - tag_name = tag_name.lower().strip() # The tag name has at least one invalid character. @@ -163,12 +155,6 @@ class TagNameConverter(Converter): "Rejecting the request.") raise BadArgument("Tag names should not be empty, or filled with whitespace.") - # The tag name is a number of some kind, we don't allow that. - elif is_number(tag_name): - log.warning(f"{ctx.author} tried to create a tag with a digit as its name. " - "Rejecting the request.") - raise BadArgument("Tag names can't be numbers.") - # The tag name is longer than 127 characters. elif len(tag_name) > 127: log.warning(f"{ctx.author} tried to request a tag name with over 127 characters. " -- cgit v1.2.3 From c0d2f51f4e4a57da23f1387e8cf6b1a9e8c02e73 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 19:24:52 +0100 Subject: Adjust tests for new converter behavior. --- tests/bot/test_converters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/bot/test_converters.py b/tests/bot/test_converters.py index b2b78d9dd..1e5ca62ae 100644 --- a/tests/bot/test_converters.py +++ b/tests/bot/test_converters.py @@ -68,7 +68,7 @@ class ConverterTests(unittest.TestCase): ('👋', "Don't be ridiculous, you can't use that character!"), ('', "Tag names should not be empty, or filled with whitespace."), (' ', "Tag names should not be empty, or filled with whitespace."), - ('42', "Tag names can't be numbers."), + ('42', "Tag names must contain at least one letter."), ('x' * 128, "Are you insane? That's way too long!"), ) -- cgit v1.2.3 From 43597788aef30381924efe8298aaa6c15f8d33f9 Mon Sep 17 00:00:00 2001 From: Joseph Date: Wed, 26 Feb 2020 19:32:34 +0000 Subject: Disable TRACE logging for Sentry breadcrumbs. --- bot/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bot/__main__.py b/bot/__main__.py index 490163739..a3f1855b5 100644 --- a/bot/__main__.py +++ b/bot/__main__.py @@ -10,7 +10,7 @@ from bot.bot import Bot from bot.constants import Bot as BotConfig, DEBUG_MODE sentry_logging = LoggingIntegration( - level=logging.TRACE, + level=logging.DEBUG, event_level=logging.WARNING ) -- cgit v1.2.3 From 5ae3949899bf8f5d637f7f5025311342012f6db6 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 20:36:36 +0100 Subject: Remove logging from tag converters. --- bot/converters.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/bot/converters.py b/bot/converters.py index 745ce5b5d..1945e1da3 100644 --- a/bot/converters.py +++ b/bot/converters.py @@ -145,26 +145,18 @@ class TagNameConverter(Converter): # The tag name has at least one invalid character. if ascii(tag_name)[1:-1] != tag_name: - log.warning(f"{ctx.author} tried to put an invalid character in a tag name. " - "Rejecting the request.") raise BadArgument("Don't be ridiculous, you can't use that character!") # The tag name is either empty, or consists of nothing but whitespace. elif not tag_name: - log.warning(f"{ctx.author} tried to create a tag with a name consisting only of whitespace. " - "Rejecting the request.") raise BadArgument("Tag names should not be empty, or filled with whitespace.") # The tag name is longer than 127 characters. elif len(tag_name) > 127: - log.warning(f"{ctx.author} tried to request a tag name with over 127 characters. " - "Rejecting the request.") raise BadArgument("Are you insane? That's way too long!") # The tag name is ascii but does not contain any letters. elif not any(character.isalpha() for character in tag_name): - log.warning(f"{ctx.author} tried to request a tag name without letters. " - "Rejecting the request.") raise BadArgument("Tag names must contain at least one letter.") return tag_name @@ -184,8 +176,6 @@ class TagContentConverter(Converter): # The tag contents should not be empty, or filled with whitespace. if not tag_content: - log.warning(f"{ctx.author} tried to create a tag containing only whitespace. " - "Rejecting the request.") raise BadArgument("Tag contents should not be empty, or filled with whitespace.") return tag_content -- cgit v1.2.3 From 97b07a8be526b62db0b1b072b4d9773bff7a8db1 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Wed, 26 Feb 2020 21:58:08 +0100 Subject: Log invalid tag names in the error handler tag fallback. --- bot/cogs/error_handler.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 3486a746c..fff1f8c9f 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -89,12 +89,20 @@ class ErrorHandler(Cog): return # Return to not raise the exception - with contextlib.suppress(BadArgument, ResponseCodeError): - await ctx.invoke( - tags_get_command, - tag_name=await TagNameConverter.convert(ctx, ctx.invoked_with) + try: + tag_name = await TagNameConverter.convert(ctx, ctx.invoked_with) + except BadArgument: + log.debug( + f"{ctx.author} tried to use an invalid command " + f"and the fallback tag failed validation in TagNameConverter." ) - return + else: + with contextlib.suppress(ResponseCodeError): + await ctx.invoke( + tags_get_command, + tag_name=tag_name + ) + return elif isinstance(e, BadArgument): await ctx.send(f"Bad argument: {e}\n") await ctx.invoke(*help_command) -- cgit v1.2.3 From ded5c543d6b7abcb9789cd3c8f097a5649270c75 Mon Sep 17 00:00:00 2001 From: "S. Co1" Date: Wed, 26 Feb 2020 16:04:17 -0500 Subject: Add clarifying comment to role checking logic implementation --- bot/cogs/antimalware.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bot/cogs/antimalware.py b/bot/cogs/antimalware.py index 957c458c0..9e9e81364 100644 --- a/bot/cogs/antimalware.py +++ b/bot/cogs/antimalware.py @@ -23,6 +23,7 @@ class AntiMalware(Cog): return # Check if user is staff, if is, return + # Since we only care that roles exist to iterate over, check for the attr rather than a User/Member instance if hasattr(message.author, "roles") and any(role.id in STAFF_ROLES for role in message.author.roles): return -- cgit v1.2.3 From f87f7559db8b352490324a535fb77e88f2f68b41 Mon Sep 17 00:00:00 2001 From: Matteo Date: Thu, 27 Feb 2020 11:37:02 +0100 Subject: Split the eval command procedure into two functions. Two functions were created: send_eval and continue_eval, in order to facilitate testing. The corresponding tests are also changed in this commit. --- bot/cogs/snekbox.py | 112 +++++++++++++++++------------- tests/bot/cogs/test_snekbox.py | 150 ++++++++++++++++++++++------------------- 2 files changed, 148 insertions(+), 114 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index efa4696b5..25b2455e8 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -179,6 +179,68 @@ class Snekbox(Cog): return output, paste_link + async def send_eval(self, ctx: Context, code: str) -> Message: + """ + Evaluate code, format it, and send the output to the corresponding channel. + + Return the bot response. + """ + async with ctx.typing(): + results = await self.post_eval(code) + msg, error = self.get_results_message(results) + + if error: + output, paste_link = error, None + else: + output, paste_link = await self.format_output(results["stdout"]) + + icon = self.get_status_emoji(results) + msg = f"{ctx.author.mention} {icon} {msg}.\n\n```py\n{output}\n```" + if paste_link: + msg = f"{msg}\nFull output: {paste_link}" + + response = await ctx.send(msg) + self.bot.loop.create_task( + wait_for_deletion(response, user_ids=(ctx.author.id,), client=ctx.bot) + ) + + log.info(f"{ctx.author}'s job had a return code of {results['returncode']}") + return response + + async def continue_eval(self, ctx: Context, response: Message) -> Tuple[bool, Optional[str]]: + """ + Check if the eval session should continue. + + First item of the returned tuple is if the eval session should continue, + the second is the new code to evaluate. + """ + _predicate_eval_message_edit = partial(predicate_eval_message_edit, ctx) + _predicate_emoji_reaction = partial(predicate_eval_emoji_reaction, ctx) + + try: + _, new_message = await self.bot.wait_for( + 'message_edit', + check=_predicate_eval_message_edit, + timeout=10 + ) + await ctx.message.add_reaction('🔁') + await self.bot.wait_for( + 'reaction_add', + check=_predicate_emoji_reaction, + timeout=10 + ) + + code = new_message.content.split(' ', maxsplit=1)[1] + await ctx.message.clear_reactions() + with contextlib.suppress(HTTPException): + await response.delete() + + except asyncio.TimeoutError: + await ctx.message.clear_reactions() + return False, None + + return True, code + @command(name="eval", aliases=("e",)) @guild_only() @in_channel(Channels.bot, hidden_channels=(Channels.esoteric,), bypass_roles=EVAL_ROLES) @@ -203,58 +265,18 @@ class Snekbox(Cog): log.info(f"Received code from {ctx.author} for evaluation:\n{code}") - _predicate_eval_message_edit = partial(predicate_eval_message_edit, ctx) - _predicate_emoji_reaction = partial(predicate_eval_emoji_reaction, ctx) - while True: self.jobs[ctx.author.id] = datetime.datetime.now() code = self.prepare_input(code) - try: - async with ctx.typing(): - results = await self.post_eval(code) - msg, error = self.get_results_message(results) - - if error: - output, paste_link = error, None - else: - output, paste_link = await self.format_output(results["stdout"]) - - icon = self.get_status_emoji(results) - msg = f"{ctx.author.mention} {icon} {msg}.\n\n```py\n{output}\n```" - if paste_link: - msg = f"{msg}\nFull output: {paste_link}" - - response = await ctx.send(msg) - self.bot.loop.create_task( - wait_for_deletion(response, user_ids=(ctx.author.id,), client=ctx.bot) - ) - - log.info(f"{ctx.author}'s job had a return code of {results['returncode']}") + response = await self.send_eval(ctx, code) finally: del self.jobs[ctx.author.id] - try: - _, new_message = await self.bot.wait_for( - 'message_edit', - check=_predicate_eval_message_edit, - timeout=10 - ) - await ctx.message.add_reaction('🔁') - await self.bot.wait_for( - 'reaction_add', - check=_predicate_emoji_reaction, - timeout=10 - ) - - log.info(f"Re-evaluating message {ctx.message.id}") - code = new_message.content.split(' ', maxsplit=1)[1] - await ctx.message.clear_reactions() - with contextlib.suppress(HTTPException): - await response.delete() - except asyncio.TimeoutError: - await ctx.message.clear_reactions() - return + continue_eval, code = await self.continue_eval(ctx, response) + if not continue_eval: + break + log.info(f"Re-evaluating message {ctx.message.id}") def predicate_eval_message_edit(ctx: Context, old_msg: Message, new_msg: Message) -> bool: diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index 112c923c8..c1c0f8d47 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -1,6 +1,7 @@ import asyncio import logging import unittest +from functools import partial from unittest.mock import MagicMock, Mock, call, patch from bot.cogs import snekbox @@ -175,28 +176,33 @@ class SnekboxTests(unittest.TestCase): async def test_eval_command_evaluate_once(self): """Test the eval command procedure.""" ctx = MockContext() - ctx.message = MockMessage() - ctx.send = AsyncMock() - ctx.author.mention = '@LemonLemonishBeard#0042' - ctx.typing = MagicMock(return_value=AsyncContextManagerMock(None)) - self.cog.post_eval = AsyncMock(return_value={'stdout': '', 'returncode': 0}) - self.cog.get_results_message = MagicMock(return_value=('Return code 0', '')) - self.cog.get_status_emoji = MagicMock(return_value=':yay!:') - self.cog.format_output = AsyncMock(return_value=('[No output]', None)) - self.bot.wait_for.side_effect = asyncio.TimeoutError + response = MockMessage() + self.cog.prepare_input = MagicMock(return_value='MyAwesomeFormattedCode') + self.cog.send_eval = AsyncMock(return_value=response) + self.cog.continue_eval = AsyncMock(return_value=(False, None)) await self.cog.eval_command.callback(self.cog, ctx=ctx, code='MyAwesomeCode') + self.cog.prepare_input.assert_called_once_with('MyAwesomeCode') + self.cog.send_eval.assert_called_once_with(ctx, 'MyAwesomeFormattedCode') + self.cog.continue_eval.assert_called_once_with(ctx, response) - ctx.send.assert_called_once_with( - '@LemonLemonishBeard#0042 :yay!: Return code 0.\n\n```py\n[No output]\n```' - ) - self.cog.post_eval.assert_called_once_with('MyAwesomeCode') - self.cog.get_status_emoji.assert_called_once_with({'stdout': '', 'returncode': 0}) - self.cog.get_results_message.assert_called_once_with({'stdout': '', 'returncode': 0}) - self.cog.format_output.assert_called_once_with('') + @async_test + async def test_eval_command_evaluate_twice(self): + """Test the eval and re-eval command procedure.""" + ctx = MockContext() + response = MockMessage() + self.cog.prepare_input = MagicMock(return_value='MyAwesomeFormattedCode') + self.cog.send_eval = AsyncMock(return_value=response) + self.cog.continue_eval = AsyncMock() + self.cog.continue_eval.side_effect = ((True, 'MyAwesomeCode-2'), (False, None)) + + await self.cog.eval_command.callback(self.cog, ctx=ctx, code='MyAwesomeCode') + self.cog.prepare_input.has_calls(call('MyAwesomeCode'), call('MyAwesomeCode-2')) + self.cog.send_eval.assert_called_with(ctx, 'MyAwesomeFormattedCode') + self.cog.continue_eval.assert_called_with(ctx, response) @async_test - async def test_eval_command_reject_two_eval(self): + async def test_eval_command_reject_two_eval_at_the_same_time(self): """Test if the eval command rejects an eval if the author already have a running eval.""" ctx = MockContext() ctx.author.id = 42 @@ -217,92 +223,98 @@ class SnekboxTests(unittest.TestCase): ctx.invoke.assert_called_once_with(self.bot.get_command("help"), "eval") @async_test - async def test_eval_command_return_error(self): - """Test the eval command error handling.""" + async def test_send_eval(self): + """Test the send_eval function.""" ctx = MockContext() ctx.message = MockMessage() ctx.send = AsyncMock() ctx.author.mention = '@LemonLemonishBeard#0042' ctx.typing = MagicMock(return_value=AsyncContextManagerMock(None)) - self.cog.post_eval = AsyncMock(return_value={'stdout': 'ERROR', 'returncode': 127}) - self.cog.get_results_message = MagicMock(return_value=('Return code 127', 'Error occurred')) - self.cog.get_status_emoji = MagicMock(return_value=':nope!:') - self.cog.format_output = AsyncMock() - self.bot.wait_for.side_effect = asyncio.TimeoutError - - await self.cog.eval_command.callback(self.cog, ctx=ctx, code='MyAwesomeCode') + self.cog.post_eval = AsyncMock(return_value={'stdout': '', 'returncode': 0}) + self.cog.get_results_message = MagicMock(return_value=('Return code 0', '')) + self.cog.get_status_emoji = MagicMock(return_value=':yay!:') + self.cog.format_output = AsyncMock(return_value=('[No output]', None)) + await self.cog.send_eval(ctx, 'MyAwesomeCode') ctx.send.assert_called_once_with( - '@LemonLemonishBeard#0042 :nope!: Return code 127.\n\n```py\nError occurred\n```' + '@LemonLemonishBeard#0042 :yay!: Return code 0.\n\n```py\n[No output]\n```' ) self.cog.post_eval.assert_called_once_with('MyAwesomeCode') - self.cog.get_results_message.assert_called_once_with({'stdout': 'ERROR', 'returncode': 127}) - self.cog.get_status_emoji.assert_called_once_with({'stdout': 'ERROR', 'returncode': 127}) - self.cog.format_output.assert_not_called() + self.cog.get_status_emoji.assert_called_once_with({'stdout': '', 'returncode': 0}) + self.cog.get_results_message.assert_called_once_with({'stdout': '', 'returncode': 0}) + self.cog.format_output.assert_called_once_with('') @async_test - async def test_eval_command_with_paste_link(self): - """Test the eval command procedure with the use of a paste link.""" + async def test_send_eval_with_paste_link(self): + """Test the send_eval function with a too long output that generate a paste link.""" ctx = MockContext() ctx.message = MockMessage() ctx.send = AsyncMock() ctx.author.mention = '@LemonLemonishBeard#0042' ctx.typing = MagicMock(return_value=AsyncContextManagerMock(None)) - self.cog.post_eval = AsyncMock(return_value={'stdout': 'SuperLongBeard', 'returncode': 0}) + self.cog.post_eval = AsyncMock(return_value={'stdout': 'Way too long beard', 'returncode': 0}) self.cog.get_results_message = MagicMock(return_value=('Return code 0', '')) self.cog.get_status_emoji = MagicMock(return_value=':yay!:') - self.cog.format_output = AsyncMock(return_value=('Truncated - too long beard', 'https://testificate.com/')) - self.bot.wait_for.side_effect = asyncio.TimeoutError - - await self.cog.eval_command.callback(self.cog, ctx=ctx, code='MyAwesomeCode') + self.cog.format_output = AsyncMock(return_value=('Way too long beard', 'lookatmybeard.com')) + await self.cog.send_eval(ctx, 'MyAwesomeCode') ctx.send.assert_called_once_with( - '@LemonLemonishBeard#0042 :yay!: Return code 0.\n\n```py\n' - 'Truncated - too long beard\n```\nFull output: https://testificate.com/' + '@LemonLemonishBeard#0042 :yay!: Return code 0.' + '\n\n```py\nWay too long beard\n```\nFull output: lookatmybeard.com' ) self.cog.post_eval.assert_called_once_with('MyAwesomeCode') - self.cog.get_status_emoji.assert_called_once_with({'stdout': 'SuperLongBeard', 'returncode': 0}) - self.cog.get_results_message.assert_called_once_with({'stdout': 'SuperLongBeard', 'returncode': 0}) - self.cog.format_output.assert_called_with('SuperLongBeard') + self.cog.get_status_emoji.assert_called_once_with({'stdout': 'Way too long beard', 'returncode': 0}) + self.cog.get_results_message.assert_called_once_with({'stdout': 'Way too long beard', 'returncode': 0}) + self.cog.format_output.assert_called_once_with('Way too long beard') @async_test - async def test_eval_command_evaluate_twice(self): - """Test the eval command re-evaluation procedure.""" + async def test_send_eval_with_non_zero_eval(self): + """Test the send_eval function with a code returning a non-zero code.""" ctx = MockContext() ctx.message = MockMessage() - ctx.message.content = '!e MyAwesomeCode' - updated_msg = MockMessage() - updated_msg .content = '!e MyAwesomeCode-2' - response_msg = MockMessage() - response_msg.delete = AsyncMock() - ctx.send = AsyncMock(return_value=response_msg) + ctx.send = AsyncMock() ctx.author.mention = '@LemonLemonishBeard#0042' ctx.typing = MagicMock(return_value=AsyncContextManagerMock(None)) - self.cog.post_eval = AsyncMock(return_value={'stdout': '', 'returncode': 0}) - self.cog.get_results_message = MagicMock(return_value=('Return code 0', '')) - self.cog.get_status_emoji = MagicMock(return_value=':yay!:') - self.cog.format_output = AsyncMock(return_value=('[No output]', None)) - self.bot.wait_for.side_effect = ((None, updated_msg), None, asyncio.TimeoutError) - - await self.cog.eval_command.callback(self.cog, ctx=ctx, code='MyAwesomeCode') - - self.cog.post_eval.assert_has_calls((call('MyAwesomeCode'), call('MyAwesomeCode-2'))) + self.cog.post_eval = AsyncMock(return_value={'stdout': 'ERROR', 'returncode': 127}) + self.cog.get_results_message = MagicMock(return_value=('Return code 127', 'Beard got stuck in the eval')) + self.cog.get_status_emoji = MagicMock(return_value=':nope!:') + self.cog.format_output = AsyncMock() # This function isn't called - # Multiplied by 2 because we expect it to be called twice - ctx.send.assert_has_calls( - [call('@LemonLemonishBeard#0042 :yay!: Return code 0.\n\n```py\n[No output]\n```')] * 2 + await self.cog.send_eval(ctx, 'MyAwesomeCode') + ctx.send.assert_called_once_with( + '@LemonLemonishBeard#0042 :nope!: Return code 127.\n\n```py\nBeard got stuck in the eval\n```' ) - self.cog.get_status_emoji.assert_has_calls([call({'stdout': '', 'returncode': 0})] * 2) - self.cog.get_results_message.assert_has_calls([call({'stdout': '', 'returncode': 0})] * 2) - self.cog.format_output.assert_has_calls([call('')] * 2) + self.cog.post_eval.assert_called_once_with('MyAwesomeCode') + self.cog.get_status_emoji.assert_called_once_with({'stdout': 'ERROR', 'returncode': 127}) + self.cog.get_results_message.assert_called_once_with({'stdout': 'ERROR', 'returncode': 127}) + self.cog.format_output.assert_not_called() + @async_test + async def test_continue_eval_does_continue(self): + """Test that the continue_eval function does continue if required conditions are met.""" + ctx = MockContext(message=MockMessage(add_reaction=AsyncMock(), clear_reactions=AsyncMock())) + response = MockMessage(delete=AsyncMock()) + new_msg = MockMessage(content='!e NewCode') + self.bot.wait_for.side_effect = ((None, new_msg), None) + + actual = await self.cog.continue_eval(ctx, response) + self.assertEqual(actual, (True, 'NewCode')) self.bot.wait_for.has_calls( - call('message_edit', check=snekbox.predicate_eval_message_edit, timeout=10), - call('reaction_add', check=snekbox.predicate_eval_emoji_reaction, timeout=10) + call('message_edit', partial(snekbox.predicate_eval_message_edit, ctx), timeout=10), + call('reaction_add', partial(snekbox.predicate_eval_emoji_reaction, ctx), timeout=10) ) ctx.message.add_reaction.assert_called_once_with('🔁') - ctx.message.clear_reactions.assert_called() - response_msg.delete.assert_called_once() + ctx.message.clear_reactions.assert_called_once() + response.delete.assert_called_once() + + @async_test + async def test_continue_eval_does_not_continue(self): + ctx = MockContext(message=MockMessage(clear_reactions=AsyncMock())) + self.bot.wait_for.side_effect = asyncio.TimeoutError + + actual = await self.cog.continue_eval(ctx, MockMessage()) + self.assertEqual(actual, (False, None)) + ctx.message.clear_reactions.assert_called_once() def test_predicate_eval_message_edit(self): """Test the predicate_eval_message_edit function.""" -- cgit v1.2.3 From afc74faadd5fb4d3fd3003bed9f2a1f241c0dc58 Mon Sep 17 00:00:00 2001 From: Matteo Date: Thu, 27 Feb 2020 11:47:01 +0100 Subject: Use unicode code point instead of literal for the snekbox re-eval emoji Unicode literals aren't really safe compared to code points --- bot/cogs/snekbox.py | 8 +++++--- tests/bot/cogs/test_snekbox.py | 6 +++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 25b2455e8..52d830fa8 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -42,6 +42,8 @@ EVAL_ROLES = (Roles.helpers, Roles.moderator, Roles.admin, Roles.owner, Roles.ro SIGKILL = 9 +REEVAL_EMOJI = '\U0001f501' # :repeat: + class Snekbox(Cog): """Safe evaluation of Python code using Snekbox.""" @@ -223,7 +225,7 @@ class Snekbox(Cog): check=_predicate_eval_message_edit, timeout=10 ) - await ctx.message.add_reaction('🔁') + await ctx.message.add_reaction(REEVAL_EMOJI) await self.bot.wait_for( 'reaction_add', check=_predicate_emoji_reaction, @@ -285,8 +287,8 @@ def predicate_eval_message_edit(ctx: Context, old_msg: Message, new_msg: Message def predicate_eval_emoji_reaction(ctx: Context, reaction: Reaction, user: User) -> bool: - """Return True if the reaction 🔁 was added by the context message author on this message.""" - return reaction.message.id == ctx.message.id and user.id == ctx.author.id and str(reaction) == '🔁' + """Return True if the reaction REEVAL_EMOJI was added by the context message author on this message.""" + return reaction.message.id == ctx.message.id and user.id == ctx.author.id and str(reaction) == REEVAL_EMOJI def setup(bot: Bot) -> None: diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index c1c0f8d47..e7a1e3362 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -303,7 +303,7 @@ class SnekboxTests(unittest.TestCase): call('message_edit', partial(snekbox.predicate_eval_message_edit, ctx), timeout=10), call('reaction_add', partial(snekbox.predicate_eval_emoji_reaction, ctx), timeout=10) ) - ctx.message.add_reaction.assert_called_once_with('🔁') + ctx.message.add_reaction.assert_called_once_with(snekbox.REEVAL_EMOJI) ctx.message.clear_reactions.assert_called_once() response.delete.assert_called_once() @@ -336,12 +336,12 @@ class SnekboxTests(unittest.TestCase): def test_predicate_eval_emoji_reaction(self): """Test the predicate_eval_emoji_reaction function.""" valid_reaction = MockReaction(message=MockMessage(id=1)) - valid_reaction.__str__.return_value = '🔁' + valid_reaction.__str__.return_value = snekbox.REEVAL_EMOJI valid_ctx = MockContext(message=MockMessage(id=1), author=MockUser(id=2)) valid_user = MockUser(id=2) invalid_reaction_id = MockReaction(message=MockMessage(id=42)) - invalid_reaction_id.__str__.return_value = '🔁' + invalid_reaction_id.__str__.return_value = snekbox.REEVAL_EMOJI invalid_user_id = MockUser(id=42) invalid_reaction_str = MockReaction(message=MockMessage(id=1)) invalid_reaction_str.__str__.return_value = ':longbeard:' -- cgit v1.2.3 From 671052ca7862fd75c38e5f5162ce0dc4ded8531b Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Thu, 27 Feb 2020 08:39:50 -0800 Subject: Moderation: fix task cancellation for permanent infraction when editing A task should not be cancelled if an infraction is permanent because tasks don't exist for permanent infractions. Fixes BOT-1V --- bot/cogs/moderation/management.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bot/cogs/moderation/management.py b/bot/cogs/moderation/management.py index f2964cd78..f74089056 100644 --- a/bot/cogs/moderation/management.py +++ b/bot/cogs/moderation/management.py @@ -129,7 +129,9 @@ class ModManagement(commands.Cog): # Re-schedule infraction if the expiration has been updated if 'expires_at' in request_data: - self.infractions_cog.cancel_task(new_infraction['id']) + # A scheduled task should only exist if the old infraction wasn't permanent + if old_infraction['expires_at']: + self.infractions_cog.cancel_task(new_infraction['id']) # If the infraction was not marked as permanent, schedule a new expiration task if request_data['expires_at']: -- cgit v1.2.3 From 848a613dee4bcbb00226fb98fc6501403dc0d7c7 Mon Sep 17 00:00:00 2001 From: Numerlor <25886452+Numerlor@users.noreply.github.com> Date: Thu, 27 Feb 2020 18:53:44 +0100 Subject: Remove unnecessary newlines from call. --- bot/cogs/error_handler.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index fff1f8c9f..0d4604430 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -98,10 +98,7 @@ class ErrorHandler(Cog): ) else: with contextlib.suppress(ResponseCodeError): - await ctx.invoke( - tags_get_command, - tag_name=tag_name - ) + await ctx.invoke(tags_get_command, tag_name=tag_name) return elif isinstance(e, BadArgument): await ctx.send(f"Bad argument: {e}\n") -- cgit v1.2.3 From 4096ef526aba41ab3fd83be16ef3b5554419d524 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Thu, 27 Feb 2020 19:43:45 -0800 Subject: Scheduler: log the exception instead of raising Logging it ourselves has a cleaner traceback and gives more control over the output, such as including the task ID. --- bot/utils/scheduling.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py index 9371dcdb7..1eae817c1 100644 --- a/bot/utils/scheduling.py +++ b/bot/utils/scheduling.py @@ -95,6 +95,9 @@ class Scheduler(metaclass=CogABCMeta): with contextlib.suppress(asyncio.CancelledError): exception = done_task.exception() - # Raise the exception if one exists. + # Log the exception if one exists. if exception: - raise exception + log.error( + f"{self.cog_name}: error in task #{task_id} {id(scheduled_task)}!", + exc_info=exception + ) -- cgit v1.2.3 From b62e15f835ff4b6c808a9b571919bcfa479d004b Mon Sep 17 00:00:00 2001 From: Matteo Date: Fri, 28 Feb 2020 09:41:12 +0100 Subject: Return only the new code in continue_eval and check for truthiness instead --- bot/cogs/snekbox.py | 13 ++++++------- tests/bot/cogs/test_snekbox.py | 8 ++++---- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 52d830fa8..381b309e0 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -209,12 +209,11 @@ class Snekbox(Cog): log.info(f"{ctx.author}'s job had a return code of {results['returncode']}") return response - async def continue_eval(self, ctx: Context, response: Message) -> Tuple[bool, Optional[str]]: + async def continue_eval(self, ctx: Context, response: Message) -> Optional[str]: """ Check if the eval session should continue. - First item of the returned tuple is if the eval session should continue, - the second is the new code to evaluate. + Return the new code to evaluate or None if the eval session should be terminated. """ _predicate_eval_message_edit = partial(predicate_eval_message_edit, ctx) _predicate_emoji_reaction = partial(predicate_eval_emoji_reaction, ctx) @@ -239,9 +238,9 @@ class Snekbox(Cog): except asyncio.TimeoutError: await ctx.message.clear_reactions() - return False, None + return None - return True, code + return code @command(name="eval", aliases=("e",)) @guild_only() @@ -275,8 +274,8 @@ class Snekbox(Cog): finally: del self.jobs[ctx.author.id] - continue_eval, code = await self.continue_eval(ctx, response) - if not continue_eval: + code = await self.continue_eval(ctx, response) + if not code: break log.info(f"Re-evaluating message {ctx.message.id}") diff --git a/tests/bot/cogs/test_snekbox.py b/tests/bot/cogs/test_snekbox.py index e7a1e3362..985bc66a1 100644 --- a/tests/bot/cogs/test_snekbox.py +++ b/tests/bot/cogs/test_snekbox.py @@ -179,7 +179,7 @@ class SnekboxTests(unittest.TestCase): response = MockMessage() self.cog.prepare_input = MagicMock(return_value='MyAwesomeFormattedCode') self.cog.send_eval = AsyncMock(return_value=response) - self.cog.continue_eval = AsyncMock(return_value=(False, None)) + self.cog.continue_eval = AsyncMock(return_value=None) await self.cog.eval_command.callback(self.cog, ctx=ctx, code='MyAwesomeCode') self.cog.prepare_input.assert_called_once_with('MyAwesomeCode') @@ -194,7 +194,7 @@ class SnekboxTests(unittest.TestCase): self.cog.prepare_input = MagicMock(return_value='MyAwesomeFormattedCode') self.cog.send_eval = AsyncMock(return_value=response) self.cog.continue_eval = AsyncMock() - self.cog.continue_eval.side_effect = ((True, 'MyAwesomeCode-2'), (False, None)) + self.cog.continue_eval.side_effect = ('MyAwesomeCode-2', None) await self.cog.eval_command.callback(self.cog, ctx=ctx, code='MyAwesomeCode') self.cog.prepare_input.has_calls(call('MyAwesomeCode'), call('MyAwesomeCode-2')) @@ -298,7 +298,7 @@ class SnekboxTests(unittest.TestCase): self.bot.wait_for.side_effect = ((None, new_msg), None) actual = await self.cog.continue_eval(ctx, response) - self.assertEqual(actual, (True, 'NewCode')) + self.assertEqual(actual, 'NewCode') self.bot.wait_for.has_calls( call('message_edit', partial(snekbox.predicate_eval_message_edit, ctx), timeout=10), call('reaction_add', partial(snekbox.predicate_eval_emoji_reaction, ctx), timeout=10) @@ -313,7 +313,7 @@ class SnekboxTests(unittest.TestCase): self.bot.wait_for.side_effect = asyncio.TimeoutError actual = await self.cog.continue_eval(ctx, MockMessage()) - self.assertEqual(actual, (False, None)) + self.assertEqual(actual, None) ctx.message.clear_reactions.assert_called_once() def test_predicate_eval_message_edit(self): -- cgit v1.2.3 From 77ee577598ae0ed9d49a0771f682e5d5a96fc7e5 Mon Sep 17 00:00:00 2001 From: Matteo Date: Fri, 28 Feb 2020 09:46:51 +0100 Subject: Ignore NotFound errors inside continue_eval It could have caused some errors if the user delete his own message --- bot/cogs/snekbox.py | 49 +++++++++++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 381b309e0..d52027ac6 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -8,7 +8,7 @@ from functools import partial from signal import Signals from typing import Optional, Tuple -from discord import HTTPException, Message, Reaction, User +from discord import HTTPException, Message, NotFound, Reaction, User from discord.ext.commands import Cog, Context, command, guild_only from bot.bot import Bot @@ -218,29 +218,30 @@ class Snekbox(Cog): _predicate_eval_message_edit = partial(predicate_eval_message_edit, ctx) _predicate_emoji_reaction = partial(predicate_eval_emoji_reaction, ctx) - try: - _, new_message = await self.bot.wait_for( - 'message_edit', - check=_predicate_eval_message_edit, - timeout=10 - ) - await ctx.message.add_reaction(REEVAL_EMOJI) - await self.bot.wait_for( - 'reaction_add', - check=_predicate_emoji_reaction, - timeout=10 - ) - - code = new_message.content.split(' ', maxsplit=1)[1] - await ctx.message.clear_reactions() - with contextlib.suppress(HTTPException): - await response.delete() - - except asyncio.TimeoutError: - await ctx.message.clear_reactions() - return None - - return code + with contextlib.suppress(NotFound): + try: + _, new_message = await self.bot.wait_for( + 'message_edit', + check=_predicate_eval_message_edit, + timeout=10 + ) + await ctx.message.add_reaction(REEVAL_EMOJI) + await self.bot.wait_for( + 'reaction_add', + check=_predicate_emoji_reaction, + timeout=10 + ) + + code = new_message.content.split(' ', maxsplit=1)[1] + await ctx.message.clear_reactions() + with contextlib.suppress(HTTPException): + await response.delete() + + except asyncio.TimeoutError: + await ctx.message.clear_reactions() + return None + + return code @command(name="eval", aliases=("e",)) @guild_only() -- cgit v1.2.3 From b8769e036e9247be47cea1b2073e92ea48724ca8 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 28 Feb 2020 09:41:37 -0800 Subject: Snekbox: mention re-evaluation feature in the command's docstring --- bot/cogs/snekbox.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bot/cogs/snekbox.py b/bot/cogs/snekbox.py index 44764e7e9..cff7c5786 100644 --- a/bot/cogs/snekbox.py +++ b/bot/cogs/snekbox.py @@ -251,7 +251,10 @@ class Snekbox(Cog): Run Python code and get the results. This command supports multiple lines of code, including code wrapped inside a formatted code - block. We've done our best to make this safe, but do let us know if you manage to find an + block. Code can be re-evaluated by editing the original message within 10 seconds and + clicking the reaction that subsequently appears. + + We've done our best to make this sandboxed, but do let us know if you manage to find an issue with it! """ if ctx.author.id in self.jobs: -- cgit v1.2.3 From 5daf1db8ea9e86568da4907d42507aa3286eb3c1 Mon Sep 17 00:00:00 2001 From: MarkKoz Date: Fri, 28 Feb 2020 09:59:57 -0800 Subject: Scheduler: correct type annotations --- bot/utils/scheduling.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bot/utils/scheduling.py b/bot/utils/scheduling.py index 1eae817c1..5760ec2d4 100644 --- a/bot/utils/scheduling.py +++ b/bot/utils/scheduling.py @@ -1,9 +1,9 @@ import asyncio import contextlib import logging +import typing as t from abc import abstractmethod from functools import partial -from typing import Dict from bot.utils import CogABCMeta @@ -17,10 +17,10 @@ class Scheduler(metaclass=CogABCMeta): # Keep track of the child cog's name so the logs are clear. self.cog_name = self.__class__.__name__ - self._scheduled_tasks: Dict[str, asyncio.Task] = {} + self._scheduled_tasks: t.Dict[t.Hashable, asyncio.Task] = {} @abstractmethod - async def _scheduled_task(self, task_object: dict) -> None: + async def _scheduled_task(self, task_object: t.Any) -> None: """ A coroutine which handles the scheduling. @@ -31,7 +31,7 @@ class Scheduler(metaclass=CogABCMeta): then make a site API request to delete the reminder from the database. """ - def schedule_task(self, task_id: str, task_data: dict) -> None: + def schedule_task(self, task_id: t.Hashable, task_data: t.Any) -> None: """ Schedules a task. @@ -51,7 +51,7 @@ class Scheduler(metaclass=CogABCMeta): self._scheduled_tasks[task_id] = task log.debug(f"{self.cog_name}: scheduled task #{task_id} {id(task)}.") - def cancel_task(self, task_id: str) -> None: + def cancel_task(self, task_id: t.Hashable) -> None: """Unschedule the task identified by `task_id`.""" log.trace(f"{self.cog_name}: cancelling task #{task_id}...") task = self._scheduled_tasks.get(task_id) @@ -65,7 +65,7 @@ class Scheduler(metaclass=CogABCMeta): log.debug(f"{self.cog_name}: unscheduled task #{task_id} {id(task)}.") - def _task_done_callback(self, task_id: str, done_task: asyncio.Task) -> None: + def _task_done_callback(self, task_id: t.Hashable, done_task: asyncio.Task) -> None: """ Delete the task and raise its exception if one exists. -- cgit v1.2.3