after_commit_everywhere icon indicating copy to clipboard operation
after_commit_everywhere copied to clipboard

Add `perform_after_commit` to ActiveJob jobs?

Open sunny opened this issue 2 years ago • 4 comments

Since it is a common use to use after_commit around ActiveJob calls (like in #19), would it make sense to add to this gem a perform_after_commit method to all ActiveJob jobs?

I use the following concern in my app:

module AfterCommitableJob
  extend ActiveSupport::Concern

  class_methods do
    def perform_after_commit(...)
      AfterCommitEverywhere.after_commit do
        perform_later(...)
      end
    end
  end
end

Would more people benefit from including this in after_commit_everywhere directly?

sunny avatar Aug 01 '23 12:08 sunny

Hey! Thanks for the suggestion.

I wonder maybe something can be done globally in ActiveJob configuration level, similar to Sidekiq's transactional_push! setting?

That way perform_later can be used unchanged.

Envek avatar Aug 02 '23 05:08 Envek

Ohhh, thank you, I didn’t know about this setting. 😍 I’ll try it out as soon as I can!

I think tying to Sidekiq specifically makes more sense since not all ActiveJob backends would benefit from using an after_commit. If your jobs are stored in a database then you would actually want them to be performed inside a transaction.

We could close this issue but before that perhaps we could document Sidekiq’s beta setting in this README? I’d be happy to submit a PR if you want.

sunny avatar Aug 04 '23 09:08 sunny

Btw, Sidekiq's setting isn't beta anymore since 7.0, but you have to enable it explicitly.

Envek avatar Aug 04 '23 12:08 Envek

Unfortunately using ActiveJob+Sidekiq the option seemed to have no effect on my side. I have opted to override perform_later altogether, which helps not introduce another method name.

module AfterCommitableJob
  extend ActiveSupport::Concern

  class_methods do
    def perform_later(...)
      AfterCommitEverywhere.after_commit do
        super
      end
    end
  end
end

sunny avatar Aug 25 '23 06:08 sunny