Skip to content
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

Support asyncio #109

Open
futursolo opened this issue Dec 4, 2015 · 19 comments
Open

Support asyncio #109

futursolo opened this issue Dec 4, 2015 · 19 comments

Comments

@futursolo
Copy link

Motor supported asyncio in its 0.5 version.

This ORM can support asyncio and/or native coroutine through Motor 0.5.

Can you provide the asyncio support?

@AlJohri
Copy link

AlJohri commented Dec 4, 2015

👍 looking forward to this!

@ilex
Copy link
Contributor

ilex commented Dec 6, 2015

Here is aiomotorengine which is fully based on motorengine. Code and tests were rewritten using the asyncio's event loop and native coroutines (using async def and await syntax, so it supports python >= 3.5 only)

@AlJohri
Copy link

AlJohri commented Dec 6, 2015

nice job!

@heynemann
Copy link
Owner

Good job guys!

@linnik
Copy link

linnik commented Dec 12, 2015

@ilex why did you made it as separate project, and not as pull request? i guess we dont need that level of fragmentation (aiomotorengine which is port of motorengine which is port of mongoengine which is trying to be like django orm)

@linnik
Copy link

linnik commented Dec 12, 2015

@ilex and if you still decide to keep separate repo anyway, i believe you should respect other contributors work and rebase repo to full commit history with patches of your own.

@AlJohri
Copy link

AlJohri commented Dec 12, 2015

I haven't looked at the implementation level, but pymongo and motor are
different libraries for a reason too. I think it might be difficult to
maintain the async version and non-async together?

  • Al

On Sat, Dec 12, 2015 at 10:29 AM, Vyacheslav Linnik <
[email protected]> wrote:

@ilex https://github.com/ilex and if you still decide to keep separate
repo anyway, i believe you should respect other contributors work and
rebase repo to full commit history with patches of your own.


Reply to this email directly or view it on GitHub
#109 (comment)
.

@linnik
Copy link

linnik commented Dec 12, 2015

Both motorengine and aiomotorengine are asynchronous: one using tornado, another using asyncio loop.
There should be one lib with two diffrerent adapters, like it's done in underlying motor library:
motor.motor_tornado.MotorClient() and motor.motor_asyncio.AsyncIOMotorClient(). Otherwise, you just splitting one ODM into two identical with only minor changes of ioloop impletementation, drastically decreasing maintainability.

@ilex
Copy link
Contributor

ilex commented Dec 12, 2015

@linnik i agree with you it's the best way but it seems to me that to meet requirement of supporting different event loops motorengine should be redesigned a bit. Because motor for example was designed to support different event loops. and i think i am really not the one who can do that at the moment.

@ilex
Copy link
Contributor

ilex commented Dec 13, 2015

@linnik i think the one way it could be done is to for example make a subfolder motorengine/asyncio/ and put here async logic mostly by inheriting classes as QuerySet and overriding async methods. Actually last week i tried to do it with connection.py and database.py so the code of motorengine/asyncio/database.py could be as follow:

import asyncio
from ..database import Database as MotorEngineDatabase


class Database(MotorEngineDatabase):

    @asyncio.coroutine
    def ping(self):
        return (yield from self.connection.admin.command('ping'))

so now we can do something like this:

import asyncio
from motorengine.asyncio import connect
io_loop = asyncio.get_event_loop()
db = connect('test', io_loop=io_loop)
io_loop.run_until_complete(db.ping())

But it still requires to apply changes to asyncio version of ping when someone change something in motorengine's version of ping

@heynemann
Copy link
Owner

I like this approach a lot better. I aggree that the fragmentation hurts the community. As a matter of fact, we have an issue with mongoengine to merge our code with theirs and support tornado in mongoengine.

@futursolo
Copy link
Author

It seemed like that I missed almost all the conversation due to my final.

In my opinion, I think if you want to integrate motorengine and aiomotorengine and make them be able to use in both tornado.ioloop.IOLoop and asyncio.BaseEventLoop, you should avoid using tornado.gen.coroutine or asyncio.coroutine since they made the code less reusable.

You can use asyncio.Future() and tornado.concurrent.Future() to make the async code compatible between tornado.ioloop.IOLoop and asyncio.BaseEventLoop because both of them has almost same API (like: set_result and set_exception). It is as easy as return right Future Object for program to yield, yield from or await.

http://www.tornadoweb.org/en/stable/concurrent.html#tornado.concurrent.Future

https://docs.python.org/3/library/asyncio-task.html#asyncio.Future

@futursolo
Copy link
Author

Also, if you have problems on working alone, we can figure out this together.

@jettify
Copy link

jettify commented Jan 2, 2016

I have never used motorengine, but have experience with motor (asyncio part), I would suggest to use separate libraries or at least completely separate modules:

  1. asyncio only >= python 3.3
  2. python3.5 has new syntax (async/await), it is painful to support both python2 and python3.5 in same code base
  3. magic to make coroutines work for both tornado and asycnio is hard to debug and support, personally I spend a day debugging issue in motor with magic coroutines and mongo replica sets.

Ideal approach is to decouple io as much as possible from protocol parsers and orm so io part could be written in any framework.

@agronholm
Copy link

The Autobahn project uses the txaio library to support both Twisted and Asyncio in the same codebase. Tornado and Asyncio are much more similar in terms of API, so this should be even easier.

@FlorianLudwig
Copy link

@heynemann could you formulate what requirements you have to get asyncio support merged into motorengine?

My colleague rebased @ilex work onto the current master of motorengine here: https://github.com/BFriedrichs/motorengine

@Midnighter
Copy link

Out of curiosity, what's the state of making a PyPI release with the asyncio modules? I'm working with the excellent Starlette and would love to see motorengine support asyncio.

@PaulRudin
Copy link

JOOI - is there any prospect of this?

@heynemann
Copy link
Owner

Guys, I don't have time to work on this. If anyone wants to join the project, I'd be more than willing to welcome new commiter(s), as long as they send a few PRs first, that is :)

Any of you guys want to get this done?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants