| Commit message (Collapse) | Author | Age | Lines |
| ... | |
| | | | | | | | | | | | |
|
| | | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | | |
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]>
|
| | | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | | |
Co-authored-by: SebastiaanZ <[email protected]>
|
| | | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | | |
Co-authored-by: SebastiaanZ <[email protected]>
|
| | | |\ \ \ \ \ \ \ \ \
| | |/ / / / / / / / /
| |/| | | | | | | | | |
|
| | | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | | |
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.
|
| | | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | | |
The function was only used in the since removed `Events` cog.
|
| | | | | | | | | | | | |
|
| | | | | | | | | | | | |
|
| | | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | | |
Adds ConfigVerifier which verifies channels when loaded.
|
| | | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | | |
Python tries to fall back on passing indices to `__getitem__` without iter implemented; failing on the first line.
|
| | | | | | | | | | | | |
|
| | | | | | | | | | | | |
|
| | | | | | | | | | | | |
|
| | | | | | | | | | | | |
|
| | | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | | |
This case is already covered by checking if at least one letter is included.
|
| | | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | | |
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.
|
| | |_|_|_|_|_|_|/ / /
|/| | | | | | | | |
| | | | | | | | | |
| | | | | | | | | | |
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.
|
| | | | | | | | | | | |
|
| | | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | | |
Logging it ourselves has a cleaner traceback and gives more control
over the output, such as including the task ID.
|
| | | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | | |
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.
|
| | | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | | |
Task.exception() only returns the exception. It still needs to be
explicitly raised.
|
| | | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | | |
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`
|
| | | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | | |
* Use imperative mood for docstring
* Explain the purpose of the parameter in the docstring
* Make log message after cog name lowercase
|
| | | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | |
| | | | | | | | | | |
The shield exists to be used for exactly this purpose so its a better
fit than create_task.
|
| | | | | | | | | |\ \
| |_|_|_|_|_|_|_|/ /
|/| | | | | | | | |
| | | | | | | | | | |
bug/backend/b754/scheduler-suppresses-errors
|
| |\ \ \ \ \ \ \ \ \ \
| |/ / / / / / / / /
|/| | | | | | | | |
| | | | | | | | | |
| | | | | | | | | | |
python-discord/bug/utils/bot-1c/reminder-unschedule-failure
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
|
| | | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
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...
|
| | | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
Because deactivate_infraction() explicitly cancels the scheduled task,
it now runs in a separate task to avoid prematurely cancelling itself.
|
| | | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
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
|
| | | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
Explicitly retrieves the task's exception, which will raise the
exception if one exists.
* Rename _suppress_cancelled_error to _handle_task_exception
|
| | | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
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.
|
| | | | | | | | | | |
|
| | | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | |
| | | | | | | | | |
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()
|
| | | | | |_|_|/ /
| | | |/| | | | |
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
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.
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
Autospec supports using AsyncMocks in 3.8 so there's no need to rely on
a subclass of CustomMockMixin for the async mocks.
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
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.
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
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".
|
| | | | | | | | |\
| |_|_|_|_|_|_|/
|/| | | | | | | |
|
| |\ \ \ \ \ \ \ \
| |_|/ / / / / /
|/| | | | | | | |
Prevent the role syncer from wiping the database table during API latency
|
| | |\ \ \ \ \ \ \
| |/ / / / / / /
|/| | | | | | |
| | | | | | | | |
bug/backend/b704/ready-missing-cache
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
* 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
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
This is more visible than it would be if it was only logged.
* Add a webhook for the dev-log channel to constants
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
Explain changes caused by 22a55534ef13990815a6f69d361e2a12693075d5.
|
| | | | | | | | | |
|
| | | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | |
| | | | | | | | |
This error is due to the use of an actual instance of APIClient as the
spec for the mock. recreate() is called in __init__ which in turn
creates a task for the _create_session coroutine.
The approach to the solution is to use the type for the spec rather than
and instance, thus avoiding any call of __init__. However, without an
instance, instance attributes will not be included in the spec.
Therefore, they are defined as class attributes on the actual APIClient
class definition and given default values.
Alternatively, a subclass of APIClient could have been made in the
tests.helpers module to define those class attributes. However, it
seems easier to maintain if the attributes are in the original class
definition.
|
| | | | | | | | | |
|