Skip to content
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

lock cannot be disabled #101

Open
molog opened this issue Aug 29, 2018 · 7 comments
Open

lock cannot be disabled #101

molog opened this issue Aug 29, 2018 · 7 comments

Comments

@molog
Copy link

molog commented Aug 29, 2018

Cannot disable lock.

Setting

redbeat_lock_key = None

is not working.

It uses "either_or" with a default value, so setting it None is useless

@jscaria
Copy link

jscaria commented Mar 28, 2020

Here's my temp fix until this gets merged into a packaged version:

In package myredbeat in schedulers.py:

class MyBeatConfig(redbeat.schedulers.RedBeatConfig):
    # Override the RedBeatConfig because it won't allow self.lock_key = None even though it's valid
    # See https://github.com/sibson/redbeat/issues/101 and https://github.com/sibson/redbeat/pull/109
    def __init__(self, app=None, single=False):
        super(MyBeatConfig, self).__init__(app)
        if single:
            self.lock_key = None
celery_app = celery.Celery("...")
celery_app.redbeat_conf = myredbeat.schedulers.MyBeatConfig(celery_app, single=False)

@yogevyuval
Copy link
Contributor

@jscaria Your solution looks promising but It's not working for me. I'm getting 'app does not have an attribute redbeat_conf' which actually makes sense. Any idea?

@jscaria
Copy link

jscaria commented May 13, 2020

@jscaria Your solution looks promising but It's not working for me. I'm getting 'app does not have an attribute redbeat_conf' which actually makes sense. Any idea?

I missed the part of overriding ensure_conf in myredbeat.schedulers. I haven't tested this recently but a quick glance suggests that's probably the issue.

@ethagnawl
Copy link

ethagnawl commented Feb 26, 2021

If the PR for the fix isn't going to be merged soon (it's been stale for a while and there are merge conflicts, so I'm guessing it won't be in the very near future) the docs should be updated to either remove references to this feature or note that it's a known issue and document any workarounds. (I'm happy to submit a PR, if it'll be considered.)

I'm seeing an issue where the lock isn't released when Redis/Celery (both 4 and 5) shut down and I'm having to manually remove either keys or the Redis DB in order to get the lock released -- this is especially problematic in my development environment. I suppose I could script that process as part of my application's bootstrap phase, but that feels heavy-handed. (I'd also like to figure out why the lock isn't being automatically released, but haven't had a chance to dig into that yet.) My setup is simple enough that I may just revert to a file-based schedule until I can either figure out the root cause of the persistent lock or this feature gets re-introduced.

UPDATE:
I'm just confirming that @jscaria's solution works for me. If the docs are updated, it should be included and/or this issue should be referenced.

YiSang665 pushed a commit to dealfinity/redbeat that referenced this issue Jun 9, 2021
YiSang665 pushed a commit to dealfinity/redbeat that referenced this issue Jun 9, 2021
YiSang665 added a commit to dealfinity/redbeat that referenced this issue Jun 9, 2021
* Fix for sibson#101 issue

* Fix for sibson#101 issue

Co-authored-by: Your Name <[email protected]>
@sibson
Copy link
Owner

sibson commented Apr 8, 2022

I'm open to accepting a doc fix or merging #206 if that delivers the correct behaviour.

@eurbs
Copy link

eurbs commented Feb 19, 2023

This is still an issue. Is there a strong preference here between the approach in #109 and #206? Additionally, what would be needed to get a fix across the finish line?

@sibson
Copy link
Owner

sibson commented Feb 19, 2023

I'm not seeing anything obvious jump out as reasons to prefer #109 or #206? If #206 merges cleanly and #109 doesn't then I'll go with #206 . Does anyone else see a reason to prefer one over the other? If someone can confirm the PR solves the problem for them I'll merge it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants