aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeLines
...
| * | | | | | | | | | Added DMs ignoring to antimalware checkGravatar ks1232020-02-26-1/+2
| | | | | | | | | | |
| * | | | | | | | | | Added to AntiMalware staff ignore check.Gravatar Karlis. S2020-02-26-1/+5
| | | | | | | | | | |
| | * | | | | | | | | Fix comparison operator when checking token expiration.Gravatar Numerlor2020-02-26-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| | * | | | | | | | | Check for falsy values instead of ``""` and `None` explicitly.Gravatar Numerlor2020-02-26-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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 <[email protected]>
| | * | | | | | | | | Specify the logged time is in UTC.Gravatar Numerlor2020-02-26-1/+1
| | | | | | | | | | |
| | * | | | | | | | | Create task for `revoke_access_token` when unloading cog to ensure it's ↵Gravatar Numerlor2020-02-26-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | executed.
| | * | | | | | | | | Make sure token exists before checking its expiration.Gravatar Numerlor2020-02-26-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Without the check and an invalid token, an AttributeError is raised; blocking the relevant ClientError from being raised in `get_access_token`.
| | * | | | | | | | | Check for empty strings alongside None before loading cog.Gravatar Numerlor2020-02-26-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Docker fetches values from the .env itself and defaults to "" instead of None, needing to do invalid access token requests before unloading itself.
| | * | | | | | | | | Restyle if body to include the error instead of adding the cog.Gravatar Numerlor2020-02-25-3/+3
| | | | | | | | | | |
| | * | | | | | | | | Fix `__iter__` for classes without subsections.Gravatar Numerlor2020-02-25-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The previous implementation assumed the config class was a subsection, failing with a KeyError if it wasn't one. Co-authored-by: kwzrd <[email protected]>
| | * | | | | | | | | Assign created task to a variable.Gravatar Numerlor2020-02-25-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Co-authored-by: SebastiaanZ <[email protected]>
| | * | | | | | | | | Wait for available guild instead of bot startup.Gravatar Numerlor2020-02-25-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Co-authored-by: SebastiaanZ <[email protected]>
| | * | | | | | | | | Merge branch 'master' into channel_constants_checkGravatar Numerlor2020-02-25-551/+1704
| | |\ \ \ \ \ \ \ \ \ | | |/ / / / / / / / / | |/| | | | | | | | |
| | * | | | | | | | | Do not attempt to load Reddit cog when environment variables are not provided.Gravatar Numerlor2020-02-24-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| | * | | | | | | | | Remove unused `chunks` function and its tests.Gravatar Numerlor2020-02-24-26/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The function was only used in the since removed `Events` cog.
| | * | | | | | | | | Change docstring mood.Gravatar Numerlor2020-02-24-1/+1
| | | | | | | | | | |
| | * | | | | | | | | Change warning text.Gravatar Numerlor2020-02-24-1/+1
| | | | | | | | | | |
| | * | | | | | | | | Add ConfigVerifier cog.Gravatar Numerlor2020-02-24-0/+41
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Adds ConfigVerifier which verifies channels when loaded.
| | * | | | | | | | | Implement `__iter__` on constants YAMLGetter.Gravatar Numerlor2020-02-24-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Python tries to fall back on passing indices to `__getitem__` without iter implemented; failing on the first line.
| | | | | | | | * | | Remove unnecessary newlines from call.Gravatar Numerlor2020-02-27-4/+1
| | | | | | | | | | |
| | | | | | | | * | | Log invalid tag names in the error handler tag fallback.Gravatar Numerlor2020-02-26-5/+13
| | | | | | | | | | |
| | | | | | | | * | | Remove logging from tag converters.Gravatar Numerlor2020-02-26-10/+0
| | | | | | | | | | |
| | | | | | | | * | | Adjust tests for new converter behavior.Gravatar Numerlor2020-02-26-1/+1
| | | | | | | | | | |
| | | | | | | | * | | Remove number check on tags.Gravatar Numerlor2020-02-26-14/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This case is already covered by checking if at least one letter is included.
| | | | | | | | * | | Pass error handler tag fallback through TagNameConverter.Gravatar Numerlor2020-02-26-2/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| | | | | | | | * | | Make sure tag name contains at least one letter.Gravatar Numerlor2020-02-26-0/+6
| |_|_|_|_|_|_|/ / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| | | | | | | | * | Scheduler: correct type annotationsGravatar MarkKoz2020-02-28-6/+6
| | | | | | | | | |
| | | | | | | | * | Scheduler: log the exception instead of raisingGravatar MarkKoz2020-02-27-2/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Logging it ourselves has a cleaner traceback and gives more control over the output, such as including the task ID.
| | | | | | | | * | Scheduler: only send warning in callback if task isn't cancelledGravatar MarkKoz2020-02-25-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| | | | | | | | * | Scheduler: properly raise task's exception the done callbackGravatar MarkKoz2020-02-25-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Task.exception() only returns the exception. It still needs to be explicitly raised.
| | | | | | | | * | Scheduler: only delete the task in the done callback if tasks are sameGravatar MarkKoz2020-02-25-16/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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`
| | | | | | | | * | Scheduler: improve cancel_task's docstringGravatar MarkKoz2020-02-25-5/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * Use imperative mood for docstring * Explain the purpose of the parameter in the docstring * Make log message after cog name lowercase
| | | | | | | | * | Moderation: use asyncio.shield to prevent self-cancellationGravatar MarkKoz2020-02-25-3/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The shield exists to be used for exactly this purpose so its a better fit than create_task.
| | | | | | | | * | Merge remote-tracking branch 'origin/master' into ↵Gravatar MarkKoz2020-02-25-1158/+2658
| | | | | | | | |\ \ | |_|_|_|_|_|_|_|/ / |/| | | | | | | | | | | | | | | | | | | bug/backend/b754/scheduler-suppresses-errors
* | | | | | | | | | Merge pull request #781 from ↵Gravatar S. Co12020-02-25-6/+11
|\ \ \ \ \ \ \ \ \ \ | |/ / / / / / / / / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | python-discord/bug/utils/bot-1c/reminder-unschedule-failure Reminders: don't cancel task if reminder is invalid when rescheduling
| * | | | | | | | | Merge branch 'master' into bug/utils/bot-1c/reminder-unschedule-failureGravatar Joseph2020-02-25-551/+1704
| |\ \ \ \ \ \ \ \ \ | |/ / / / / / / / / |/| | | | | | | | |
| * | | | | | | | | Reminders: don't cancel task if reminder is invalid when reschedulingGravatar MarkKoz2020-02-23-6/+11
| |/ / / / / / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| | | | | | | * | Scheduler: make _scheduled_tasks privateGravatar MarkKoz2020-02-15-6/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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...
| | | | | | | * | Moderation: avoid prematurely cancelling deactivation taskGravatar MarkKoz2020-02-15-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Because deactivate_infraction() explicitly cancels the scheduled task, it now runs in a separate task to avoid prematurely cancelling itself.
| | | | | | | * | Scheduler: cancel the task in the callbackGravatar MarkKoz2020-02-15-12/+30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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
| | | | | | | * | Scheduler: raise task exceptions besides CancelledErrorGravatar MarkKoz2020-02-15-4/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Explicitly retrieves the task's exception, which will raise the exception if one exists. * Rename _suppress_cancelled_error to _handle_task_exception
| | | | | | | * | Scheduler: remove loop parameter from schedule_taskGravatar MarkKoz2020-02-15-16/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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.
| | | | | | | * | Scheduler: correct schedule_task's docstringGravatar MarkKoz2020-02-15-1/+1
| | | | | | | | |
| | | | | | | * | Scheduler: remove create_task functionGravatar MarkKoz2020-02-15-15/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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()
| | | | | | | * | Scheduler: fix #754 - only suppress CancelledErrorGravatar MarkKoz2020-02-15-6/+7
| | | | |_|_|/ / | | | |/| | | |
| | | | | | | * Use MagicMock as return value for _get_diff mockGravatar Sebastiaan Zeeff2020-02-24-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The `_get_diff` method of TestSyncer class is mocked using an AsyncMock object. By default, when an AsyncMock object is called **and awaited**, it returns a child mock of the same time (another AsyncMock) according to the "the child is a like the parent" principle. This means that the _get_diff method will return an AsyncMock unless a different return_value is explicitly provided. Because of that "child is like parent" behavior, this will happen in lines 194-196 of bot.cogs.sync.syncers (annotations added by me): ``` // `diff` will be a child AsyncMock as "child is like parent" diff = await self._get_diff(guild) // `diff._asdict` will be an AsyncMock as "child is like parent" and, // after being called, it will return an unawaited coroutine object // we assign the name `diff_dict`: diff_dict = diff._asdict() // `diff_dict` is still an unawaited coroutine object meaning that it // doesn't have an `items()` method: totals = {k: len(v) for k, v in diff_dict.items() if v is not None} ``` Original, unannotated: https://github.com/python-discord/bot/blob/c81a4d401ea434e98b0a1ece51d3d10f1a3ad226/bot/cogs/sync/syncers.py#L194-L196 This will lead to the following exception when running the tests: ```py ====================================================================== ERROR: test_sync_confirmation_context_redirect (tests.bot.cogs.sync.test_base.SyncerSyncTests) (ctx=None, author=<MockMember name='mock.user' spec_set='Member' id='140583452034864'>, message=None) If ctx is given, a new message should be sent and author should be ctx's author. ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/sebastiaan/pydis/repositories/bot/tests/bot/cogs/sync/test_base.py", line 348, in test_sync_confirmation_context_redirect await self.syncer.sync(guild, ctx) File "/home/sebastiaan/pydis/repositories/bot/bot/cogs/sync/syncers.py", line 196, in sync totals = {k: len(v) for k, v in diff_dict.items() if v is not None} AttributeError: 'coroutine' object has no attribute 'items' ``` The solution is to assign an explicit return value so the parent mock doesn't "guess" and return an object of its own type. I previously did that by providing a specific `_Diff` object as the return value, but I should have gone with a `MagicMock` to signify that it's not an important return value; it's just something that needs to support/mimic the API we use on it. So that's what this commit adds.
| | | | | | | * Sync tests: use autospec instead of MockSyncerGravatar MarkKoz2020-02-23-18/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Autospec supports using AsyncMocks in 3.8 so there's no need to rely on a subclass of CustomMockMixin for the async mocks.
| | | | | | | * Migrate syncers test suite to Python 3.8Gravatar Sebastiaan Zeeff2020-02-24-75/+34
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The test suite for the new role/member syncers used the "old"-style test suite with the helpers implemented for Python 3.7. I have migrated it to use the new Python 3.8 asyncio test helpers.
| | | | | | | * Use realistic mixin implementationGravatar Sebastiaan Zeeff2020-02-24-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Instead of using the mixin class bare, I've now included into a class tha subclasses unittest.TestCase as that's how it's going to be used "in the wild".
| | | | | | | * Merge branch 'master' into python38-migrationGravatar Sebastiaan Zeeff2020-02-24-541/+1658
| | | | | | | |\ | |_|_|_|_|_|_|/ |/| | | | | | |