Motor -> Async PyMongo #1113
Motor -> Async PyMongo #1113
Conversation
roman-right
force-pushed
the
roman-right/async-pymongo
branch
from
ed26bba to
36a3452
Compare
roman-right
requested review from
CAPITAINMARVEL,
adeelsohailahmed and
staticxterm
|
Why do you want to migrate? |
|
Hi @dantetemplar , |
There was a problem hiding this comment.
Hi, great to see you are back and to see these changes!
I also tried making the same changes to see how much work it is, but then got stuck on the "cursor" changes...
Apologies in advance for the number of review comments. Please note that I didn't test this out on some production code, only have done a visual inspection of the code. I plan to test this out with the proposed changes.
PyMongo Async is still in Beta according to MongoDB official documentation, so I propose to wait for an official release as well, and as you already stated.
Also, these changes should result in a next Major version release of Beanie due to changes to Beanie API.
| @@ -15,7 +15,6 @@ To set the root model you have to set `is_root = True` in the inner Settings cla | |||
|
|
|||
| ```py hl_lines="20 20" | |||
| from typing import Optional, List | |||
| from motor.motor_asyncio import AsyncIOMotorClient | |||
There was a problem hiding this comment.
Missing import for AsyncMongoClient:
from pymongo import AsyncMongoClient
| @@ -163,7 +164,7 @@ async def run_migration_class( | |||
| db = DBHandler.get_db() | |||
| if client is None: | |||
| raise RuntimeError("client must not be None") | |||
| async with await client.start_session() as s: | |||
| async with client.start_session() as s: | |||
There was a problem hiding this comment.
| async with client.start_session() as s: | |
| with client.start_session() as s: |
According to PyMongo documentation, a simple "with" context manager is used.
| @@ -118,7 +118,7 @@ async def commit(self) -> Optional[BulkWriteResult]: | |||
| raise ValueError( | |||
| "The document model class must be specified before committing operations." | |||
| ) | |||
| return await self.object_class.get_motor_collection().bulk_write( | |||
| return await self.object_class.get_pymongo_collection().bulk_write( | |||
There was a problem hiding this comment.
Since this is a breaking change in itself (renaming the public interface method) should this simply be renamed to get_collection()?
| @@ -640,7 +640,7 @@ def list_to_index_model(left: List[IndexModelField]): | |||
| return [index.index for index in left] | |||
|
|
|||
| @classmethod | |||
| def from_motor_index_information(cls, index_info: dict): | |||
| def from_pymongo_index_information(cls, index_info: dict): | |||
There was a problem hiding this comment.
Same here, should we drop "motor"/"pymongo" from method name and simply rename to from_index_information() ?
| @@ -12,8 +12,8 @@ def get_settings(cls) -> ItemSettings: | |||
| pass | |||
|
|
|||
| @classmethod | |||
| def get_motor_collection(cls) -> AsyncIOMotorCollection: | |||
| return cls.get_settings().motor_collection | |||
| def get_pymongo_collection(cls) -> AsyncCollection: | |||
There was a problem hiding this comment.
Since this is a breaking change, could it be renamed to simply get_collection()?
| @@ -193,8 +193,8 @@ async def test_index_dropping_is_not_allowed(db): | |||
| allow_index_dropping=False, | |||
| ) | |||
|
|
|||
| collection: AsyncIOMotorCollection = ( | |||
| DocumentTestModelWithComplexIndex.get_motor_collection() | |||
| collection: AsyncCollection = ( | |||
There was a problem hiding this comment.
| collection: AsyncCollection = ( | |
| collection = ( |
| @@ -217,8 +217,8 @@ async def test_index_dropping_is_not_allowed_as_default(db): | |||
| document_models=[DocumentTestModelWithDroppedIndex], | |||
| ) | |||
|
|
|||
| collection: AsyncIOMotorCollection = ( | |||
| DocumentTestModelWithComplexIndex.get_motor_collection() | |||
| collection: AsyncCollection = ( | |||
There was a problem hiding this comment.
| collection: AsyncCollection = ( | |
| collection = ( |
| @@ -397,6 +397,6 @@ class Settings: | |||
| # To force collection creation | |||
| await NewDocument(test_str="Roman Right").save() | |||
|
|
|||
| collection: AsyncIOMotorCollection = NewDocument.get_motor_collection() | |||
| collection: AsyncCollection = NewDocument.get_pymongo_collection() | |||
There was a problem hiding this comment.
| collection: AsyncCollection = NewDocument.get_pymongo_collection() | |
| collection = NewDocument.get_pymongo_collection() |
| @@ -206,7 +206,7 @@ class Config: | |||
|
|
|||
| def __init__(self, *args: Any, **kwargs: Any) -> None: | |||
| super(Document, self).__init__(*args, **kwargs) | |||
| self.get_motor_collection() | |||
| self.get_pymongo_collection() | |||
There was a problem hiding this comment.
This line feels out of place. It does nothing.
| connection_string | ||
| ).get_default_database() | ||
|
|
||
| self.database: AsyncIOMotorDatabase = database | ||
| self.database: AsyncDatabase = database | ||
|
|
||
| if multiprocessing_mode: | ||
| self.database.client.get_io_loop = asyncio.get_running_loop |
There was a problem hiding this comment.
There is no method "get_io_loop" on the "client"/AsyncMongoClient anymore. This is another breaking change.
Also, is there any documentation on what this "multiprocessing_mode" parameter is, and how can it be used?
Switch from Motor to Async PyMongo
Possible breaking change: Some internal fields and method names have been updated to remove references to
Motor.