Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add perform_after_commit to ActiveJob jobs? #27

Open
sunny opened this issue Aug 1, 2023 · 4 comments
Open

Add perform_after_commit to ActiveJob jobs? #27

sunny opened this issue Aug 1, 2023 · 4 comments

Comments

@sunny
Copy link

sunny commented Aug 1, 2023

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?

@Envek
Copy link
Owner

Envek commented Aug 2, 2023

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.

@sunny
Copy link
Author

sunny commented Aug 4, 2023

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.

@Envek
Copy link
Owner

Envek commented Aug 4, 2023

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

@sunny
Copy link
Author

sunny commented Aug 25, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants