search

05bit / peewee-async

Asynchronous interface for peewee ORM powered by asyncio
http://peewee-async-lib.readthedocs.io
MIT License
733 stars 100 forks source link

Add signal support for async #96

Open x0139 opened 6 years ago

x0139 commented 6 years ago

Please add Signal playhouse support on insert method to work with await objects.create()

rudyryk commented 6 years ago

OK, this is up to discuss. Unfortunately, implementation could be tricky because signals are designed to be sync and we need async. And also if someone added sync signals they won't be called from async code. This could be even worse than having no support for signals at all.

rudyryk commented 6 years ago

However, we could try calling sync signals with run_in_executor().

x0139 commented 6 years ago

There is small problem to call run_in_executor() - await objects.create() uses insert, and in playhouse peewee said: Peewee signals do not work when you use the Model.insert(), Model.update(), or Model.delete()

x0139 commented 6 years ago

Why signals with async is a bad idea?

rudyryk commented 6 years ago

I'm not saying this is a bad idea. I just point that it may cause unexpected behaviour and need to be designed properly. Consider someone has added signals in their sync codebase. Should we trigger those signals or not? If we do that, than we have to use run_in_executor(). And yes, we should replicate the logic from sync version as close as possible.