peewee-async icon indicating copy to clipboard operation
peewee-async copied to clipboard

Add signal support for async

Open x0139 opened this issue 6 years ago • 5 comments

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

x0139 avatar Jul 02 '18 14:07 x0139

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 avatar Jul 09 '18 10:07 rudyryk

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

rudyryk avatar Jul 09 '18 10:07 rudyryk

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 avatar Jul 10 '18 06:07 x0139

Why signals with async is a bad idea?

x0139 avatar Jul 10 '18 06:07 x0139

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.

rudyryk avatar Jul 10 '18 08:07 rudyryk