-
Notifications
You must be signed in to change notification settings - Fork 73
Asyncio Module Ported more tests [4/6] #750
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 Module Ported more tests [4/6] #750
Conversation
- Fixed a bug where an asyncio Lock was tried to be acquired without a release - Store asyncio tasks in order not to lose them - Removed start/shutdown from AsyncioReactor
…integration-tests2
…integration-tests2
…integration-tests2
…integration-tests2
| return await self._context.proxy_manager.destroy_proxy(self.service_name, self.name) | ||
| async with asyncio.TaskGroup() as tg: # type: ignore[attr-defined] | ||
| tg.create_task(self._on_destroy()) | ||
| return await tg.create_task( |
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.
why not use simpler (really need concurrency here?):
await self._on_destroy()
return await self._conftext...
Reads a little odd with the return there -- is that even safe?
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.
It's safe, the task group will not exit without all tasks are completed.
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.
It looks odd to me to have the return there, the following seems clearer IMO as it doesn't mix the task group completion semantics with some nested return.
async with asyncio.TaskGroup() as tg: # type: ignore[attr-defined]
tg.create_task(self._on_destroy())
t = await tg.create_task(...)
return t.result()
# Conflicts: # hazelcast/internal/asyncio_connection.py # hazelcast/internal/asyncio_reactor.py
No description provided.