-
Notifications
You must be signed in to change notification settings - Fork 206
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
asyncio integration #20
Conversation
Unix sockets
Tail and SON manipulation tests
asyncio motor collections tests
Motor pool for asyncio
port database test for asyncio
port cursor tests
Move to streams
Port tests from test_motor_basic
@asvetlov this is incredibly exciting! I'll begin a review on Monday & Tuesday but I may not finish before I go on vacation, August 18-24. Is that ok or will it cause any delays for you? |
That's totally fine but I'll be glad to have quick partial feedback at least. |
|
||
# TODO: with stack_context.NullContext(): |
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.
Can you restore this TODO so I remember to examine it? I'm concerned that if an exception-handler in one Tornado coroutine returns the socket to the pool, another Tornado coroutine that receives the socket will be in the wrong exception-handling context.
Gorgeous. I have a few minor quibbles. I expect I'll merge this soon after I return from vacation around August 25. Thanks so much. |
I've opened issues for everything I need to do once this is merged, you can see them in the list of issues that MOTOR-40 depends on. Shall I merge this now or is there additional work you'd like to do in your fork before I merge? Of course, once I merge I invite you to continue fixing anything you'd like to fix, in separate pull requests. |
Cool! On Wed, Aug 26, 2015 at 4:50 PM A. Jesse Jiryu Davis <
|
@ajdavis please review