-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
ref(rules): Fix task name for delayed rule processing #70831
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #70831 +/- ##
=======================================
Coverage 80.01% 80.02%
=======================================
Files 6504 6505 +1
Lines 290769 290781 +12
Branches 50119 50119
=======================================
+ Hits 232666 232684 +18
+ Misses 57666 57660 -6
Partials 437 437
|
@@ -73,6 +73,7 @@ def callback(self, buffer_hook_event: BufferHookEvent, data: RedisBuffer) -> boo | |||
try: | |||
callback = self._registry[buffer_hook_event] | |||
except KeyError: | |||
logger.info("buffer_hook_event.missing", extra={"key_name": buffer_hook_event.value}) |
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.
nit: might be nice to make this an error log instead of an info log, and pass the exception.
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'll change it in a separate PR, really wanna get this out to do testing
Follow up from #70831 (comment) to make this raise an exception rather than just log an error.
The task for processing delayed rules wasn't imported into
CELERY_IMPORTS
and the name was incorrect - this fixes both and adds a log message if the buffer hook registry isn't working as expected.