Skip to content

Conversation

wesleykendall
Copy link
Contributor

The first version of getting a mutex lock from the DB in Django

@wesleykendall
Copy link
Contributor Author

@jaredlewis @joshmarlow this is the solution for django mutex locking. Look it over and let me know if you have any concerns or suggestions. Would like to deploy this for our celery jobs today.

README.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: in practice and in example, we should decorate this with @abc.abstract method. The advantage is that we will get an error even trying to instantiate subclasses that do not override this; the NotImplementedError will only throw an exception when we try to call run_worker (which would not occur if we accidentally override run).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah i agree with josh, I would try to use the abstract class. Also with this could we just put the decorator on the run?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea I was aware of the abc thing, but I just used this as a quick example. I'll update it to use abc here in the docs though

@joshmarlow
Copy link
Contributor

@wesleykendall I had a few suggestions. Looks great to me 🐫

One final thought: perhaps create a testing script to test it under load; create lots of threads/processes that all try in parallel to create the same lock to generate an error? Haven't thought it out in any great detail...

@jaredlewis
Copy link
Contributor

@wesleykendall this looks amazing. Very well done

@jaredlewis
Copy link
Contributor

🐗

joshmarlow added a commit that referenced this pull request Mar 7, 2014
@joshmarlow joshmarlow merged commit b03ff17 into ambitioninc:develop Mar 7, 2014
somewes added a commit that referenced this pull request Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants