PYTHON-4864 - Create async version of SpecRunnerThread #2094
Conversation
There was a problem hiding this comment.
not loving the duplicated code between sync and async but i'm guessing its because the async version needs some more methods? If so, then i understand and can live with it >.<
There was a problem hiding this comment.
The async version doesn't implement threading.Thread, but it still needs to match the same API as the synchronous version. Let me see if I can reduce some of the duplication though.
There was a problem hiding this comment.
Did some refactoring, much less duplication now. Great call-out!
| if _IS_SYNC: | ||
| await self.cond.wait(10) # type: ignore[call-arg] | ||
| else: | ||
| await asyncio.wait_for(self.cond.wait(), timeout=10) # type: ignore[arg-type] |
There was a problem hiding this comment.
Let's use our _async_cond_wait compat function here to avoid the branching.
| """Run the 'runOnThread' operation.""" | ||
| thread = self.entity_map[spec["thread"]] | ||
| thread.schedule(lambda: self.run_entity_operation(spec["operation"])) | ||
| await thread.schedule(functools.partial(self.run_entity_operation, spec["operation"])) |
There was a problem hiding this comment.
Are we running any async unified tests that use runOnThread?
There was a problem hiding this comment.
The SDAM unified tests are the only ones that use runOnThread. Those are currently slated to be converted to async, yes.
| def __init__(self, name): | ||
| super().__init__() | ||
|
|
||
| class ConcurrentRunner(PARENT): |
There was a problem hiding this comment.
Let's move this to utils.py since it will be used by non-spec tests.
There was a problem hiding this comment.
Since utils.py is not currently synchro'd, let's delay that move to PYTHON-5113, it'll involve a lot of git changes.
There was a problem hiding this comment.
Okay then what about helpers.py?
| self.name = name | ||
| self.stopped = False |
There was a problem hiding this comment.
Stopped is never set to True by this class.
No description provided.