aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeLines
...
| | * | | | | | | | | 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
| | | | | | | |\ | |_|_|_|_|_|_|/ |/| | | | | | |
* | | | | | | | Merge pull request #711 from python-discord/bug/backend/b704/ready-missing-cacheGravatar Mark2020-02-23-536/+1657
|\ \ \ \ \ \ \ \ | |_|/ / / / / / |/| | | | | | | Prevent the role syncer from wiping the database table during API latency
| * | | | | | | Merge remote-tracking branch 'origin/master' into ↵Gravatar MarkKoz2020-02-23-621/+997
| |\ \ \ \ \ \ \ | |/ / / / / / / |/| | | | | | | | | | | | | | | bug/backend/b704/ready-missing-cache
| * | | | | | | Sync: code style refactoringGravatar MarkKoz2020-02-23-7/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * Convert diff namedtuple to dict outside the dict comprehension * Define long condition as a boolean instead of in the if statement * Pass role and user dicts to aiohttp normally instead of unpacking
| * | | | | | | Bot: send empty cache warning to a webhookGravatar MarkKoz2020-02-21-3/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This is more visible than it would be if it was only logged. * Add a webhook for the dev-log channel to constants
| * | | | | | | API: add comment explaining class attributesGravatar MarkKoz2020-02-19-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Explain changes caused by 22a55534ef13990815a6f69d361e2a12693075d5.