|  | 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. |