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

Fix ActiveJob::EnqueueAfterTransactionCommit API #51525

Merged
merged 1 commit into from Apr 10, 2024

Conversation

casperisfine
Copy link
Contributor

Fix: #51426 (comment)

perform_later is supposed to return the Job instance on success, and false on error.

When the enqueue is automatically delayed, it's of course impossible to predict if the actual queueing will succeed, but for backward compatibility reasons, it's best to assume it will.

If necessary, you can hold onto the job instance and check for #successfully_enqueued? after the transaction has completed.

cc @matthewd @bensheldon

@casperisfine casperisfine force-pushed the aj-after-commit-return-value branch 3 times, most recently from 05257c6 to 81dcc82 Compare April 9, 2024 15:35
@@ -28,11 +30,15 @@ module EnqueueAfterTransactionCommit # :nodoc:
end

if after_transaction
ActiveRecord.after_all_transactions_commit(&block)
self.successfully_enqueued = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dunno if the successfully_enqueued = true is really justified. Maybe what we'd want is to return the job but with successfully_enqueued = false?

Fix: rails#51426 (comment)

`perform_later` is supposed to return the Job instance on success,
and `false` on error.

When the `enqueue` is automatically delayed, it's of course impossible
to predict if the actual queueing will succeed, but for backward compatibility
reasons, it's best to assume it will.

If necessary, you can hold onto the job instance and check for
`#successfully_enqueued?` after the transaction has completed.
@byroot byroot merged commit 41d867d into rails:main Apr 10, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants