PYTHON-5081 Convert test.test_gridfs to async#2099
PYTHON-5081 Convert test.test_gridfs to async#2099sleepyStick merged 15 commits intomongodb:masterfrom
Conversation
gridfs/asynchronous/grid_file.py
Outdated
| doc = await anext(cursor) | ||
| return AsyncGridOut(self._collection, file_document=doc, session=session) | ||
| except StopIteration: | ||
| except (StopIteration, StopAsyncIteration): |
There was a problem hiding this comment.
| except (StopIteration, StopAsyncIteration): | |
| except StopAsyncIteration: |
The asynchronous API should only throw StopAsyncIteration, and the synchronous API should only throw StopIteration.
There was a problem hiding this comment.
oh yeah, that makes sense HAHA silly me
test/asynchronous/test_gridfs.py
Outdated
| assert data == b"hello" | ||
| else: | ||
|
|
||
| class JustWrite(asyncio.Task): |
There was a problem hiding this comment.
We don't want to subclass Task, but have the async JustWrite class have a self.task instance.
There was a problem hiding this comment.
oh right, that's what we said today in OH just changed it :)
test/asynchronous/test_gridfs.py
Outdated
| self.n = n | ||
| self.daemon = True | ||
|
|
||
| class JustRead(asyncio.Task): |
test/asynchronous/test_gridfs.py
Outdated
|
|
||
| class JustWrite: | ||
| def __init__(self, fs, n): | ||
| async def run(): |
There was a problem hiding this comment.
This should be a separate method, not nested inside __init__.
test/asynchronous/test_gridfs.py
Outdated
|
|
||
| class JustRead: | ||
| def __init__(self, fs, n, results): | ||
| async def run(): |
test/utils.py
Outdated
| async def asyncjoinall(tasks): | ||
| """Join tasks with a 5-minute timeout, assert joins succeeded""" | ||
| for t in tasks: | ||
| await asyncio.wait_for(t.task, 300) |
There was a problem hiding this comment.
| await asyncio.wait_for(t.task, 300) | |
| await asyncio.wait([t.task], timeout=300) |
wait_for will throw a TimeoutError if it times out, wait will just return the task that didn't complete.
There was a problem hiding this comment.
ahh i didn't know that!
NoahStapp
left a comment
NoahStapp
left a comment
There was a problem hiding this comment.
LGTM pending Shane's review.
test/utils.py
Outdated
| async def asyncjoinall(tasks): | ||
| """Join tasks with a 5-minute timeout, assert joins succeeded""" | ||
| for t in tasks: | ||
| await asyncio.wait([t.task], timeout=300) |
There was a problem hiding this comment.
Let's use gather() to join all tasks at once.
There was a problem hiding this comment.
Better yet, use asyncio.wait.
There was a problem hiding this comment.
good idea! done!
test/test_gridfs.py
Outdated
|
|
||
| if _IS_SYNC: | ||
|
|
||
| class JustWrite(threading.Thread): |
There was a problem hiding this comment.
Can we use the shared thread/task api that Noah introduced here?
There was a problem hiding this comment.
yes! Just changed that!
test/asynchronous/test_gridfs.py
Outdated
| if _IS_SYNC: | ||
| joinall(tasks) | ||
| else: | ||
| await asyncio.wait([t.task for t in tasks if t.task is not None]) |
There was a problem hiding this comment.
Let's put this logic into an async_joinall method to avoid this branch. Also, there should be a 5-minute timeout on this wait to match joinall.
There was a problem hiding this comment.
makes sense, done!
test/asynchronous/test_gridfs.py
Outdated
| if _IS_SYNC: | ||
| joinall(tasks) | ||
| else: | ||
| await asyncio.wait([t.task for t in tasks if t.task is not None]) |
| "test_load_balancer.py", | ||
| "test_json_util_integration.py", | ||
| "test_gridfs_spec.py", | ||
| "test_json_util_integration.py", |
There was a problem hiding this comment.
test_json_util_integration is duplicated here.
There was a problem hiding this comment.
nice catch, sorry for missing that!
3 participants
No description provided.