-
Notifications
You must be signed in to change notification settings - Fork 0
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
Context manager for selecting content DB #3
Conversation
@@ -0,0 +1,133 @@ | |||
""" |
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.
This is the file where most of the work is done.
@jamalex There were some weird conflicts here, as I think I had rebased my branch. Cherry picked it in, but unfortunately stole the blame for one of the commits in the merge conflict resolution :/ Is in my branch now. |
Oh, ok -- I did some rebasing against your Note on this stuff: it works for most cases. One problem (which I largely mitigated, but might still surface) is that Django's DB routing gets called at the time a queryset is resolved, not the time it's created, so if a queryset is created inside the context manager and then returned to elsewhere before lazy resolution occurs, it's possible it will throw an error. I think the cases in which this would happen should be rare now, with some changes I made and through upgrading to the bleeding edge version of django-mptt that includes this fix from a week ago: django-mptt/django-mptt#478 |
Yeah - I have further mitigated this issue by removing any mention of specific databases within the content API methods - so now they have to be called from within a context manager. The one case where this has still cropped up is in the Django REST framework Serializers, where a queryset is returned from methods and then further processing done. Eli has worked around this by continuing to use the |
refactor button styling a bit
updates based on outstanding PR
updated docs around dependencies and virtualenv
Fixes & Buildkite for Android
Add public channel endpoints and tests
Summary
Adds a context manager/decorator to select content databases across a given set of database operations.
TODO