MOTOR-938 Docs for watch() incorrectly call ChangeStream.close() #163
Conversation
blink1073
changed the title
motor/core.py
Outdated
| if change_stream is not None: | ||
| change_stream.close() | ||
| asyncio.run(change_stream.close()) |
There was a problem hiding this comment.
Can you confirm that the close() is not needed? I believe the async with collection.watch() as change_stream line will handle closing the cursor when KeyboardInterrupt is raised to asycnio.run().
There was a problem hiding this comment.
I did some local testing, ensuring that the change_stream was actually closed. asyncio.run properly runs __aexit__(), closing the stream. For the tornado loop, I had to manually run the close operation.
| if change_stream is not None: | ||
| change_stream.close() | ||
| loop.add_callback(change_stream.close) |
There was a problem hiding this comment.
Does this actually work? I would expect close to never run since the loop is not running at this point. What if we use loop.run_sync similar to asyncio.run?:
loop = tornado.ioloop.IOLoop.current()
# Start watching collection for changes.
try:
loop.run_sync(lambda: watch(collection))
except KeyboardInterrupt:
pass
| finally: | ||
| if change_stream is not None: | ||
| change_stream.close() | ||
| pass |
There was a problem hiding this comment.
Does this pass need to be here?
There was a problem hiding this comment.
Yes, that or a comment, or it would be a syntax error.
No description provided.