Add Redis backend and automatic expiry to MongoDB #32
Conversation
|
Updated pull request with the following:
|
This change adds Redis as an alternative storage backend.
Implementation notes:
- The database name (which is a number) should be provided in the
connection URI.
- The keys are prefixed with the 'collection' string, so that it is
possible to share the same database with other applications.
- All values are stored as JSON documents ({"value": value}) in order to
try to keep the original data type, since Redis stores all values as bytes.
Note however that it is still possible that the retrieved object differs
from the stored one. (Dictionaries with non-string keys are an example.)
- All values are stored together with a TTL. The TTL is None by default,
which means that the record never expires. The TTL value can be set
during initialisation.
This commit changes the storage tests to use 'fakeredis' and 'mongomock'
instead of trying to launch a real MongoDB server. Expiration tests
have been added, too.
| @@ -12,6 +12,7 @@ | |||
| description='OpenID Connect Provider (OP) library in Python.', | |||
| install_requires=[ | |||
| 'oic >= 0.15.0', | |||
| 'pymongo' | |||
| 'pymongo', | |||
| 'redis' | |||
There was a problem hiding this comment.
I am thinking that we should put pymongo and redis as extras_require. Then users can choose the preferred backend/db.
There was a problem hiding this comment.
Do you want it to be two try: import pymongo except ImportError: pass blocks in storage.py?
There was a problem hiding this comment.
I think that is the simplest change. I'm fine with that.
There was a problem hiding this comment.
Please check the third commit in the patchset. Do you think that the documentation should be updated as well?
There was a problem hiding this comment.
I went ahead and merged this, but adding documentation would be great ;) of course it can be done in a separate PR. Thank you!
| @@ -12,6 +12,7 @@ | |||
| description='OpenID Connect Provider (OP) library in Python.', | |||
| install_requires=[ | |||
| 'oic >= 0.15.0', | |||
| 'pymongo' | |||
| 'pymongo', | |||
| 'redis' | |||
There was a problem hiding this comment.
I think that is the simplest change. I'm fine with that.
This changes the record layout in MongoDB so that the modification time is now stored as a datetime object instead of a timestamp. It enables us to create an auto-expiry index on that field. Note that expiring objects is an asynchronous task in MongoDB. Unlike Redis, MongoDB uses collection-level expiry settings, therefore it is not possible to create multiple wrapper objects that operate on the same collection with different TTL values. Additionally, there is a minor change that allows specifying the database name in the URI instead of an argument. In case the name is specified both in the URI and the argument, the one in the argument is silently ignored.
|
Hey Both, just here to let you know that this pull request feature looks great, and is something that we are really looking forward to. We currently rely on https://pypi.org/project/redis-collections/ as storage backend. However, this doesn't support the expiration on keys as implemented in this pull request. If there is anything we can do to help progress this PR, please let us know. Thanks! |
Fail only if the module required by the storage wrapper is not available. Since the library is usable without any persistent storage driver at all, the database-specific module requirements have been moved to extras_require.
|
Added a pull request in bajnokk#2 to add some additional fixes in two files + an update to the latest master branch of this repo. |
The first of the two patches adds Redis storage backend to pyop. It also makes it possible to assign the storage wrapper automatically based on the connection URL prefix, for example
mongodb://localhostis routed to MongoWrapper andredis://localhost/1is routed to RedisWrapper.Redis supports per-record automatic expiry, therefore a TTL is added to every record on write operations.
Since expiry is also possible for MongoDB, there is a second patch that implements the automatic expiry for MongoWrapper (also fixes #27). Note that this changes the record layout (
modified_tsbecomeslast_modified), therefore if external tools were reading the MongoDB records, these must follow the changes.Storage tests have been extended to include the Redis implementation, too. Since the automatic setup of the MongoDB database does not work for me (perhaps the
mongodcommand line arguments inconftest.pyare outdated?), I have only tested it with a manually started MongoDB instance. Redis tests usefakeredis, therefore they do not need a real database.Questions & suggestions are welcome.