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

Support ActiveJob 7.2 enqueue_after_transaction_commit #79

Merged

Conversation

joshuay03
Copy link
Contributor

@joshuay03 joshuay03 commented Apr 15, 2024

What is the purpose of this pull request?

Ref: rails/rails#51426

Disables the throwing of Isolator::BackgroundJobError for jobs queued from within transactions if the new ActiveJob config enqueue_after_transaction_commit is enabled.

What changes did you make? (overview)

Updated the active_job adapter to be disabled if enqueue_after_transaction_commit is enabled globally or by the configured queue adapter.

Is there anything you'd like reviewers to focus on?

Checklist

  • I've added tests for this change
  • I've added a Changelog entry
  • I've updated a documentation

@joshuay03 joshuay03 force-pushed the support-enqueue-after-transaction-commit branch from 7dd998f to ca88e36 Compare April 15, 2024 11:41
@joshuay03 joshuay03 changed the title Support ActiveRecord 7.2 enqueue_after_transaction_commit Support ActiveRecord 7.2 enqueue_after_transaction_commit Apr 15, 2024
@joshuay03 joshuay03 changed the title Support ActiveRecord 7.2 enqueue_after_transaction_commit Support ActiveJob 7.2 enqueue_after_transaction_commit Apr 15, 2024
@joshuay03 joshuay03 force-pushed the support-enqueue-after-transaction-commit branch 2 times, most recently from 4cf3cd6 to 5ddaac8 Compare April 15, 2024 11:49
@@ -143,3 +145,5 @@ This, for example, makes Isolator compatible with Rails multi-database apps.
[@Mange]: https://github.com/Mange
[@tomgi]: https://github.com/tomgi
[@tagirahmad]: https://github.com/tagirahmad
[@arthurWD]: https://github.com/arthurWD
Copy link
Contributor Author

@joshuay03 joshuay03 Apr 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed in #77

@joshuay03 joshuay03 marked this pull request as ready for review April 15, 2024 11:55
@joshuay03 joshuay03 force-pushed the support-enqueue-after-transaction-commit branch from 5ddaac8 to d50ea62 Compare April 18, 2024 13:23
Copy link
Owner

@palkan palkan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, check the question regarding the callback.

Also, I've fixed the CI problem with sqlite3 in master; could you please rebase?

lib/isolator/adapters/background_jobs/active_job.rb Outdated Show resolved Hide resolved
@joshuay03 joshuay03 force-pushed the support-enqueue-after-transaction-commit branch 2 times, most recently from aca583d to 7eeba75 Compare April 19, 2024 23:36
@joshuay03 joshuay03 requested a review from palkan April 19, 2024 23:38
@joshuay03 joshuay03 force-pushed the support-enqueue-after-transaction-commit branch from 7eeba75 to c91c351 Compare April 23, 2024 09:40
@joshuay03 joshuay03 force-pushed the support-enqueue-after-transaction-commit branch from c91c351 to 1a07957 Compare April 23, 2024 09:41
@palkan
Copy link
Owner

palkan commented Apr 26, 2024

Thanks! I'll take care of the mailer specs (looks like we need to cover both pre 7.2 and post 7.2 cases).

@palkan palkan merged commit 7d0dbc0 into palkan:master Apr 26, 2024
9 of 10 checks passed
@joshuay03 joshuay03 deleted the support-enqueue-after-transaction-commit branch April 26, 2024 22:11
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

Successfully merging this pull request may close these issues.

2 participants