New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add MongoStorage for FSM (MongoDB driver) #1407
base: dev-3.x
Are you sure you want to change the base?
Conversation
Signed-off-by: selfkilla666 <selfkilla666@yahoo.com>
Signed-off-by: selfkilla666 <selfkilla666@yahoo.com>
Signed-off-by: selfkilla666 <selfkilla666@yahoo.com>
Signed-off-by: selfkilla666 <selfkilla666@yahoo.com>
Signed-off-by: selfkilla666 <selfkilla666@yahoo.com>
✔️ Changelog found.Thank you for adding a description of the changes |
Signed-off-by: selfkilla666 <selfkilla666@yahoo.com>
Signed-off-by: selfkilla666 <selfkilla666@yahoo.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about events isolation mechanism based on the Mongo? It can be safely used?
from aiogram.fsm.storage.mongo import AIOGRAM_DATABASE_NAME | ||
|
||
|
||
MONGO_TEST_URI: str = "mongodb+srv://bot:25sFG3vj7H2T4j5R@mongostorage-aiogram-te.pugxfxq.mongodb.net/?retryWrites=true&w=majority" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WTF? Why was this connection string hardcoded? It should be configured outside of tests, just like Redis.
client.close() | ||
|
||
|
||
class TestMongoStorage: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to implement additional test-cases for the new storage.
Just implement fixture for Mingo Storage like this: https://github.com/aiogram/aiogram/blob/dev-3.x/tests/conftest.py#L47-L62
and then add fixture usage for already defined test-cases: https://github.com/aiogram/aiogram/blob/dev-3.x/tests/test_fsm/storage/test_storages.py#L14
Additional tests is needed only for specific methods that is not covered by other tests.
from aiogram.fsm.state import State | ||
from aiogram.fsm.storage.base import BaseStorage | ||
from aiogram.fsm.storage.base import StateType | ||
from aiogram.fsm.storage.base import StorageKey |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this import lines was separated? It can be grouped.
try: | ||
from motor.motor_asyncio import AsyncIOMotorClient | ||
from motor.motor_asyncio import AsyncIOMotorDatabase | ||
from motor.motor_asyncio import AsyncIOMotorCollection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same question with imports.
_client: AsyncIOMotorClient | ||
_database: AsyncIOMotorDatabase | ||
_states_collection: AsyncIOMotorCollection | ||
_data_collection: AsyncIOMotorCollection |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this attributes type-hinted in two places at the same time?
You don't need to write type-hints on the class level if you specify it in the init method or vice versa.
if isinstance(value, str): | ||
return value | ||
if isinstance(value, State): | ||
return value.state |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if value has any other type than NoneType
, str
or State
?
) | ||
|
||
@staticmethod | ||
def resolve_state(value) -> None | str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
value should be typed
|
||
updated_data: dict = await self.get_data(key) | ||
updated_data.update(data) | ||
await self.set_data(key, updated_data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can implement update method in a more safe way with Mongo instead of doing two separated operations.
@@ -61,6 +61,9 @@ fast = [ | |||
redis = [ | |||
"redis[hiredis]~=5.0.1", | |||
] | |||
mongo = [ | |||
"motor>=3.3.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Motor 4.x could potentially break the aiogram, so you should use stricter conditions for version selection.
Trailing comma should also be used here.
@@ -0,0 +1 @@ | |||
Adding a new class :code:`aiogram.fsm.storage.MongoStorage` - MongoDB driver for aiogram's Finite State Machine. It works on the base of asynchronous :code:`motor` module to work with MongoDB. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Added
instead of Adding
word.
Write something like this:
Adding a new class :code:`aiogram.fsm.storage.MongoStorage` - MongoDB driver for aiogram's Finite State Machine. It works on the base of asynchronous :code:`motor` module to work with MongoDB. | |
Added new storage :code:`aiogram.fsm.storage.MongoStorage` for Finite State Machine based on Mongo DB (using :code:`motor` library) |
@selfkilla666, any news? |
Mongo Storage seems to have been cursed 😞 3 unfinished PR's with this storage implementation. |
Description
Adding a new class
aiogram.fsm.storage.MongoStorage
- MongoDB driver for aiogram's Finite State Machine. It works on the base of asynchronousmotor
module to work with MongoDB. Was developed as part of an in-house project, and finalized to add to the aiogram library. Can be used via:or initialize directly for the URI:
Otherwise, the module works like all other FSM-Storage. Documentation and dependency updates are also included.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
This FSM storage module for MongoDB was written for in-house development (bot), and was tested for performance in a business project. Only documentation, dependencies, and black-style code reformatting were added to add it to the standard
aiogram
library. There are no ready unit tests available (yet).Test Configuration:
Operating System: Windows 10 / Ubuntu 22.04 LTS
Python version: 3.12
Checklist:
Update: I added a unit test to check MongoStorage and it passes successfully