fix: register logger_task and webhook_task in CELERY_IMPORTS#9413
Open
pranav-afk wants to merge 1 commit into
Open
fix: register logger_task and webhook_task in CELERY_IMPORTS#9413pranav-afk wants to merge 1 commit into
pranav-afk wants to merge 1 commit into
Conversation
API middleware enqueues plane.bgtasks.logger_task.process_logs on each request, but the worker never imported the module, so tasks were discarded with KeyError. Explicitly import logger_task (and webhook_task for the same fragile transitive-import pattern) so Celery registers them. Fixes makeplane#9357 Fixes makeplane#8928
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCelery’s ChangesCelery task registration
Estimated code review effort: 1 (Trivial) | ~3 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Registers Celery background tasks that the API enqueues but the worker never loaded, so those jobs were silently dropped.
APITokenLogMiddleware queues plane.bgtasks.logger_task.process_logs on API requests. The task module existed, but it was missing from CELERY_IMPORTS in plane/settings/common.py, so the worker never registered it and logged:
Received unregistered task of type 'plane.bgtasks.logger_task.process_logs'
KeyError: 'plane.bgtasks.logger_task.process_logs'
That broke API activity / audit logging and produced noisy worker errors. The same pattern applies to webhook_task (only registered via fragile transitive imports), so both modules are listed explicitly.
Change: add to CELERY_IMPORTS:
• plane.bgtasks.logger_task
• plane.bgtasks.webhook_task
Type of Change
• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] Feature (non-breaking change which adds functionality)
• [ ] Improvement (change that would cause existing functionality to not work as expected)
• [ ] Code refactoring
• [ ] Performance improvements
• [ ] Documentation update
Test Scenarios
• Restart the Celery worker after deploy and confirm it starts without import errors
• Hit any authenticated API endpoint that goes through APITokenLogMiddleware
• Confirm worker logs no longer show unregistered process_logs tasks
• Confirm process_logs tasks are consumed (or API activity logs are written as expected)
• Optionally trigger a webhook delivery and confirm webhook tasks still run
References
• Fixes #9357
• Fixes #8928
Summary by CodeRabbit