-
-
Notifications
You must be signed in to change notification settings - Fork 535
[13.0][IMP] queue_job: check that queue_job_notify trigger exists in db #264
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
Conversation
|
Hi @guewen, |
queue_job/jobrunner/runner.py
Outdated
| ("queue_job", "queue_job_notify"), | ||
| ) | ||
| if cr.fetchone()[0] != 3: # INSERT, DELETE, UPDATE | ||
| _logger.debug( |
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.
Since having the module installed without the trigger is never normal, should we log with warn or error level?
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.
Good catch.... IMO we should log with error level.
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.
makes sense! fixed
d67c111 to
3dceec3
Compare
guewen
left a comment
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.
Thanks!
sbidoul
left a comment
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. Good idea, thanks!
|
/ocabot merge patch |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at 35fbf96. Thanks a lot for contributing to OCA. ❤️ |
Could save time in cases such as #262 :)