| Commit message (Collapse) | Author | Age | Lines |
| ... | |
| | | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | | |
executed.
|
| | | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | | |
Without the check and an invalid token, an AttributeError is raised; blocking the relevant ClientError from being raised in `get_access_token`.
|
| | | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | | |
Docker fetches values from the .env itself and defaults to "" instead of None, needing to do invalid access token requests before unloading itself.
|
| | | | | | | | | | | | |
|
| | | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | |
| | | | | | | | | | | |
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.
|