-
-
Notifications
You must be signed in to change notification settings - Fork 4
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
Beat unique fix #6
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #6 +/- ##
==========================================
+ Coverage 58.74% 60.00% +1.25%
==========================================
Files 7 7
Lines 286 295 +9
==========================================
+ Hits 168 177 +9
Misses 118 118
☔ View full report in Codecov by Sentry. |
Hey, first of all - awesome work on this package. Secondly, I've noticed an issue with this approach - namely, when you call the decorated function directly, the lock is still acquired (while it shouldn't in this case) and it is then never released, because it doesn't use any of the task api calls/signals. I've subclassed the HeimdallTask to handle this. Basically, bypassing the acquire lock behaviour if the task/function is called directly.
|
Great catch! It may still be desirable to have the lock used when running directly, so we'll need to make this a configuration option and move cleanup into the call. We'll add this and a test for it in the refactor in #10. |
It's possible for the
celery beat
scheduler to skip callingapply_async()
, if it's unable to import the actual task. In these cases, it'll resort to usingsend_task()
, see https://github.com/celery/celery/blob/main/celery/beat.py#L406. This means that our unique lock may never be checked, since we hook it intoTask.apply_async()
.This change adds verification of the lock at the time a task is run, while keeping the early lock as well.
This is a known issue with other similar libraries, see: