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
Don't log enqueuing details when the job wasn't enqueued #49515
Conversation
Thanks for the PR! Could you please squash your commits? 🙇 |
@zzak Thanks for taking a look! Apologies for not squashing before opening the PR. That's been done now 👍🏻 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, @bensheldon wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the change looks good in the context of avoiding the error and is a definite improvement.
In terms of "is it the ideal error message for perform_now?" it also makes me think: the log message should also exclude the queue name. But that looks harder to do and that could be looked into separately.
👍
@bensheldon That's totally fair, and something I was considering when making this change. I agree that it would be a bigger change to do this correctly. With this change I'm inferring that the job wasn't enqueued with I'd love to explore that more, but I think that doing so separately is probably the way to go. |
Don't log enqueuing details when the job wasn't enqueued
Motivation / Background
This Pull Request was created to fix a small bug introduced in #48066.
Detail
This changes the logged message when a job is started. Before, the logged message always attempted to include the
enqueued_at
info. Unfortunately, for jobs that are run withperform_now
, they are not enqueued and instead executed immediately. This leftenqueued_at
asnil
which in turn caused aNoMethodError
to be raised when attempting to callutc
on thenil
value.Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]