search

rails / solid_queue

Database-backed Active Job backend
MIT License
1.66k stars 90 forks source link

Added enqueue_after_transaction_commit setting #209

Closed tannakartikey closed 2 months ago

tannakartikey commented 3 months ago

The enqueue_after_transaction_commit method was recently added to ActiveJob to be supported by the adapters. PR: https://github.com/rails/rails/pull/51426

rosa commented 3 months ago

@tannakartikey thank you so much for this! It was in my to-do list so this is super helpful 😊

I was thinking, however, about setting this by default to true 🤔 I think it's safer for most people and goes more in the spirit of this change in Active Job.

If setting it to true by default, we'd need to modify this section in the README because the behaviour it warns about would be disabled now.

tannakartikey commented 3 months ago

@tannakartikey thank you so much for this! It was in my to-do list so this is super helpful 😊

I was thinking, however, about setting this by default to true 🤔 I think it's safer for most people and goes more in the spirit of this change in Active Job.

If setting it to true by default, we'd need to modify this section in the README because the behavior it warns about would be disabled now.

Glad to hear that, @rosa. Thank you for reviewing.

I have set it to true by default and updated README. Let me know what you think. Thanks

zerobearing2 commented 2 months ago

Any idea when this might get merged? This issue has blocked us from staying on edge rails atm.

rosa commented 2 months ago

Hey @zerobearing2, I'll merge now, sorry for the delay. I've been working on other stuff and travelling, and after the first review, I failed to get back to it to review the newer changes 😳

This issue has blocked us from staying on edge rails atm.

How so? Couldn't you just have set config.active_job.enqueue_after_transaction_commit to always or never? 🤔

Anyway, I'll merge this now. Thanks so much again, @tannakartikey, the README change looks great.

zerobearing2 commented 2 months ago

How so? Couldn't you just have set config.active_job.enqueue_after_transaction_commit to always or never? 🤔

We kept getting missing method error for solid queue adapter due to missing enqueue_after_transaction_commit? method. Wasn't aware setting enqueue_after_transaction_commit to always or never would skip the check. :thinking: Thanks for merging though, cheers!