| Commit message (Collapse) | Author | Lines |
|---|
|
|
|
- Refactor error messages in _to_typestring and _from_typestring to just
print the prefix tuples instead of that custom error string.
- Create a RedisKeyOrValue type to simplify some annotations.
- Simplify partialmethod calls.
- Make the signatures for _to_typestring and _from_typestring one-liners
- Fix a typo in the errors.
|
|
I should be shot.
|
|
We're using functools.partialmethod to make the code a little cleaner
and more readable here.
Read more about them here:
https://docs.python.org/3/library/functools.html#functools.partial
https://docs.python.org/3/library/functools.html#functools.partialmethod
|
|
Also added a test for this.
This is the DRYest approach I could find. It's a little ugly, but I
think it's probably good enough.
|
|
|
|
This check was no longer being used anywhere, having been replaced by
in_whitelist_check.
|
|
The way we were doing the asyncio.Lock() stuff for increment was
slightly problematic. @aeros has adviced us that it's better to just
initialize the lock as None in __init__, and then initialize it inside
the first coroutine that uses it instead. This ensures that the correct
loop gets attached to the lock, so we don't end up getting errors like
this one:
RuntimeError: got Future <Future pending> attached to a different loop
This happens because the lock and the actual calling coroutines aren't
on the same loop. When creating a new test, test_increment_lock, we
discovered that we needed a small refactor here and also in the test
class to make this new test pass.
So, now we're creating a DummyCog for every test method, and this will
ensure the loop streams never cross. Cause we all know we must never
cross the streams.
|
|
|
|
For example, we don't want the mod commands to produce any kind of error
message when run by ordinary users in regular channels - these should
have the perception of being invisible and unavailable.
|
|
|
|
|
|
|
|
We're moving the actual predicate into the `utils.checks` folder, just
like we're doing with most of the other decorators. This is to allow us
the flexibility to use it as a pure check, not only as a decorator.
This commit doesn't actually change any functionality, just moves it
around.
|
|
|
|
This just tests that the various RuntimeErrors are reachable - that
includes the error about not having a bot instance, the one about not
being a class attribute, and the one about not having instantiated the
class.
This test addresses a concern raised by @MarkKoz in a review.
I've decided not to test that actual contents of these RuntimeErrors,
because I believe that sort of testing is a bit too brittle. It
shouldn't break a test just to change the content of an error string.
|
|
|
|
Thanks @MarkKoz!
|
|
|
|
|
|
If you passed a key for a value that was the same as your optional, it
would just return it but not delete it. This edge case isn't worth it,
so I'm just removing that condition and letting the extra API call fly.
|
|
Also, refactor error messages to be consistent and DRY throughout the
file.
|
|
|
|
Useful for those that run redis with docker-compose but not the bot.
The bot on the host won't have access to the Docker network in such
case so the port must be exposed.
|
|
|
|
This is a simple validation that only check the type of the collection.
It does not validate the types inside the collection because that has
proven to be quite complex.
|
|
Note that `Optional[x]` is just an alias for `Union[None, x]` so this
effectively supports `Optional` too.
This was especially troublesome because the redis password must be
unset/None in order to avoid authentication, but the test would complain
that `None` isn't a `str`. Setting to an empty string would pass the
test but then make redis authenticate and fail.
|
|
|
|
Changed a RuntimeError to a KeyError (thanks @MarkKoz), and also added
some tests to ensure that the right errors are raised whenever this
method is used incorrectly.
|
|
Without this option, all contributors would need to set up a Redis
server in order to run the bot. But with use_fakeredis set to True, this
is no longer necessary because it will just set up a fakeredis redis
pool instead of trying to contact an actual server.
This is more than good enough for most local testing purposes, since
data persistence across restarts isn't really relevant for them.
This also means we need to move fakeredis into our real dependency list
instead of having it as a dev dependency, so there's a minor change for
that as well.
I also made a small kaizen change to sort all the dependencies in the
Pipfile alphabetically.
|
|
Forgot to update the additional_spec_asyncs when changing the name of
this Bot attribute to be public.
|
|
Mostly trace and exception logging.
|
|
It's a public attribute, we're accessing it from RedisCache.
|
|
Sometimes, we just want to store a counter in the cache. In this case,
it is convenient to have a single method that will allow us to increment
or decrement this counter.
These methods allow you to decrement or increment floats and integers by
an specified amount. By default, it'll increment or decrement by 1.
Since this involves several API requests, we create an asyncio.Lock so
that we don't end up with race conditions.
|
|
There really was no compelling reason why this method should return an
AsyncIterator or than that `async for items in cache.items()` has nice
readability, but there were a few concerns. One is a concern about race
conditions raised by @SebastiaanZ, and @MarkKoz raised a concern that it
was misleading to have an AsyncIterator that only "pretended" to be
lazy.
To address these concerns, I've refactored it to return a regular
ItemsView instead.
I also improved the docstring, and fixed the relevant tests.
|
|
We provide suggestions for how to solve these problems now.
|
|
Thanks to @MarkKoz for suggesting most of these in their code review.
|
|
It's leaner to just move that code out of the class and up to the module
level as constants.
This commit also renames ValidRedisType to RedisType.
|
|
Co-authored-by: Mark <[email protected]>
|
|
Instead of relying on __str__ representation,
we'll use the __name__ dunder.
Co-authored-by: Mark <[email protected]>
|
|
|
|
It's not feasible to mock it because all the commands return futures
rather than being coroutines, so they cannot automatically be turned
into AsyncMocks. Furthermore, no code should ever use the redis session
directly besides RedisCache. Since the tests for RedisCache already use
fakeredis, there's no use in trying to mock redis in MockBot.
|
|
Thanks to @kwzrd for this idea, basically we're making a constant with
the typestring prefixes and iterating that in all our converters.
These converter functions will also now raise TypeErrors if we try
to convert something that isn't in this constants list.
I've also added a new test that tests this functionality.
|
|
Previously we would try to .delete keys that did not exist if a default
was provided when calling .pop. This is okay to do (because HDEL will
just ignore any attempts to delete non-existing calls), but it does add
an additional pointless API call to Redis, so I've added some validation
as a small optimization.
This also adds a few additional lines of documentation as requested by
@SebastiaanZ in their review.
|
|
|
|
This was incorrectly suggesting the user needed to
create an instance of RedisCache, when in fact it is
the parent that needs to be instantiated.
Co-authored-by: Sebastiaan Zeeff <[email protected]>
|
|
|
|
|
|
|
|
|
Leon Sandøy
Dennis Pham
Joseph Banks
MarkKoz
Leon Sandøy