"atomic"-ness of hooks should be configureable or removed about django-lifecycle HOT 6 CLOSED

On Sun, Jan 30, 2022, 2:38 PM Anthony McClosky ***@***.***> wrote: First of all, thanks for this library. The API is really well done. However, today I discovered that in #85 <#85> the change was made to force hooks to run inside of a transaction, which for many cases is desirable behavior, however one of my uses for lifecycle hooks is to queue background jobs in POST_SAVE assuming any calls to the model's save() will either observe the default django orm autocommit behavior or abide by whatever the behavior set by its current context will be. Forcing a transaction / savepoint that wraps all model hooks using the atomic decorator means you can't safely make a call to an external service(or queue a celery / rq job) and assume the model changes will be visible. For example, it is not unusual for a background job to begin execution before the transaction that queued the job commits. I'd be happy to open a PR that either reverts #85 <#85> or makes the current behavior configureable in some way depending no your preference if you are open to it. — Reply to this email directly, view it on GitHub <#97>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB32I5WFHE6WFIFLE7EQT5TUYWOVZANCNFSM5NEWIORQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

On Sun, Jan 30, 2022, 2:50 PM Anthony McClosky ***@***.***> wrote: I'm looking at it right now. Making it globally configureable looks straightforward which is probably what I will open a PR for. The "better" solution may experiment with would be to add a new set of hooks, POST_*, that complements the existing AFTER_* hooks which execute only after a transaction succeeds. — Reply to this email directly, view it on GitHub <#97 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB32I5WKYV72Y4EQKA2WAVDUYWP7VANCNFSM5NEWIORQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you commented.Message ID: ***@***.***>

On Sun, Jan 30, 2022, 2:52 PM Robert Singer ***@***.***> wrote: o I like the "post transact" hook idea too, if that seems simple to fit into the code base, we could go that route On Sun, Jan 30, 2022, 2:50 PM Anthony McClosky ***@***.***> wrote: > I'm looking at it right now. > > Making it globally configureable looks straightforward which is probably > what I will open a PR for. > > The "better" solution may experiment with would be to add a new set of > hooks, POST_*, that complements the existing AFTER_* hooks which execute > only after a transaction succeeds. > > — > Reply to this email directly, view it on GitHub > <#97 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AB32I5WKYV72Y4EQKA2WAVDUYWP7VANCNFSM5NEWIORQ> > . > Triage notifications on the go with GitHub Mobile for iOS > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> > or Android > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. > > You are receiving this because you commented.Message ID: > ***@***.***> >