Published at Feb. 28, 2017
| Tagged with:
Python
,
Django
,
django-admin
A weird race condition
Updated solution, please check below!
Some background:
We have a model that is edited only via the Django admin. The save method of the model fires a celery task to update several other records. The reason for using celery here is that the amount of related objects can be pretty big and we decided that it is best to spawn the update as a background job. We have unit and integration tests, the code was also manually tested, everything looked nice and we deployed it.
On the next day we found out that the code is acting weird. No errors, everything looked like it has worked but the updated records actually contained the old data before the change. The celery task accepts the object ID as an argument and the object is fetched from the database before doing anything else so the problem was not that we were passing some old state of the object. Then what was going on?
Trying to reproduce it:
Things are getting even weirder. The issues is happening every now and then.
Hm...? Race condition?! Let's take a look at the code:
class MyModel(models.Model):
def save(self, **kwargs):
is_existing_object = False if self.pk else True
super(MyModel, self).save(**kwargs)
if is_existing_object:
update_related_objects.delay(self.pk)
The bigger picture: (this is simplified version of what is happening for the full one check Django's source)
def changeform_view(...):
with transaction.atomic():
...
self.save_model(...) # here happens the call to MyModel.save()
self.save_related(...)
...
Ok, so the save is actually wrapped in transaction. This explains what is going on. Before the transaction is committed the updated changes are not available for the other connections. This way when the celery task is called we end up in a race condition whether the task will start before or after the transaction is completed. If celery manages to pick the task before the transaction is committed it reads the old state of the object and here is the error.
Solution (updated): The best solution is to use transaction.on_commit. This way the call to the celery task will be executed only after the transaction is completed succesfully. Also, if you call the method outside of transaction the function will be executed immediately so it will also work if you are saving the model outside the admin. The only downside is that this functionality has been added to Django in version 1.9. So it wasn't an option for us. Still, special thanks to Jordan Jambazov for pointing this approach to me, I'll definitely use it in the future.
Unfortunately we are using Django 1.8 so we picked a quick and ugly fix. We added a 60 seconds countdown to the task call giving the transaction enough time to complete. As the call to the task depends on some logic and which properties of the models instance are changes moving it out of the save method was a problem. Another option could be to pass all the necessary data to the task itself but we decided that it will make it too complicated.
However I am always open to other ideas so if you have hit this issue before I would like to know how you solved it.
Published at July 14, 2011
| Tagged with:
Python
,
Django
,
django-forms
,
django-cms
,
django-admin
,
django-models
,
usability
,
Development
,
Django CMS
... how to make usable drop-downs with Django CMS pages
Problem: some times when you create custom applications or plugins for Django CMS you need a property that connects the current item to a page in the CMS. Nothing simple than this - you just add a ForeignKey in your model that points to the Page model and everything is (almost)fine. Example:
from cms.models import Page
class MyModel(models.Model):
# some model attributes here
page = models.ForeignKey(Page)
Cool right? Not exactly. The problem is that these pages are in hierarchical structure and listing them in a flat list may be/is little confusing. So let's indent them accordingly to their level in the hierarchy.
Solution: The easies way to achieve this indentation is to overwrite the choices list of the ForeignKey field in the ModelForm __init__ method.
class MyModelForm(forms.ModelForm):
class Meta:
model = MyModel
def __init__(self, *args, **kwargs):
super(MyModelForm, self).__init__(*args, **kwargs)
choices = [self.fields['page'].choices.__iter__().next()]
for page in self.fields['page'].queryset:
choices.append(
(page.id, ''.join(['-'*page.level, page.__unicode__()]))
)
self.fields['page'].choices = choices
The magic lies between lines 7 and 11, on line 7 we create a list with one element the default empty option for the drop down. The need to use "__iter__().next()" comes from the fact that the choices attribute of the fields is django.forms.models.ModelChoiceIterator object which is iterable, but not indexable i.e. you can not just use self.fields['url'].choices[0].
After we had the empty choice now it is time to add the real ones, so we iterate over the queryset(8th line) that holds them and for each item we add a tuple to our choices list(10). The first item of the tuple is the page id - nothing special, but the second one... here the python magic comes. We multiple the minus sign('-') by the page level and join the result with the page title. The only thing left is to replace the field choices(line 12) and here is the result:
Final words: For me this is much more usable than the flat list. Of course you can modify the queryset to return only published pages or filter the results in other way and still use the identation code from above.
I'll be happy to hear your thoughts on this.
Published at June 1, 2011
| Tagged with:
Python
,
Django
,
django-forms
,
dynamic values
,
django-admin
,
django-models
,
ForeignKey
,
Development
... or how to limit dynamic foreign key choices in both edit drop-down and admin filter.
The problem: Having a foreign key between models in django is really simple. For example:
class Question(models.Model):
# some fields here
class Answer(models.Model):
# some fields here
question = models.ForeignKey(Question)
from django.contrib.admin.filterspecs import FilterSpec, RelatedFilterSpec
class CustomFilterSpec(RelatedFilterSpec):
def __init__(self, *args, **kwargs):
super(CustomFilterSpec, self).__init__(*args, **kwargs)
self.lookup_choices = get_questions() #this method returns the dynamic list
FilterSpec.filter_specs.insert(0, (lambda f: bool(f.rel and hasattr(f, 'custom_filter_spec')), CustomFilterSpec))
class Answer(models.Model):
# some fields here
question = models.ForeignKey(Question)
question.custom_filter_spec = True # this is used to identify the fields which use the custom filter
Final word: The solutions is pretty simple and really easy to implement but should be used carefully. If your filtering method(in my case get_questions) is slow/resource consuming it may bring you troubles. Here is the place where and you should think about caching it. This is a place where a application cache can be used. Hope this will help you as much as it helped me.
Published at Dec. 12, 2010
| Tagged with:
Django
,
django-cms
,
django-admin
,
django-models
,
Development
,
Django CMS
... or how to add some fields to the Page model in the admin without changing Django CMS core
The problem: Some times the Page model just lack of something you need. In my case this was "page avatars". Long story short - I needed an image/avatar for each page in my CMS. So what I have to do was to add a field to the Page model that will keep the info about the avatar path.
Speciality: Of course the simplest solution was just to edit the file(pagemodel.py) that contain the Page model. Unfortunately in the common case(and in my too) several different application resides on a single server and share the same Django CMS. So changing the core code is inconvenient because every change on it will affect all applications that rely on the same CMS core. So what? Copying the whole Django CMS files in my app and modifying them? This does not seems like a good solution too. Fortunately there is a simple way to do that and leave the core code unchanged.
Solution: First you need to create a new model that will hold the new fields you need and a foreign key to the Page model.
from django.db import models
from django.utils.translation import ugettext_lazy as _
from cms.models.pagemodel import Page
from filebrowser.fields import FileBrowseField as FBF
class PageAvatars(models.Model):
page = models.ForeignKey(Page, unique=True, verbose_name=_("Page"),
editable=False, related_name='extended_fields')
big_avatar = FBF(_(u'Big Avatar'), max_length=255, blank=True)
small_avatar = FBF(_(u'Small Avatar'), max_length=255, blank=True)
from extended_pages.models import PageAvatars
from cms.admin.pageadmin import PageAdmin
from cms.models.pagemodel import Page
from django.contrib import admin
class PageAvatarsAdmin(admin.TabularInline):
model = PageAvatars
PageAdmin.inlines.append(PageAvatarsAdmin)
admin.site.unregister(Page)
admin.site.register(Page, PageAdmin)
The last two lines(11 and 12) are used to force Django to reload the Page model admin.
Now you have your new field as an inline in the Page admin.
Final words: As you can see the solution is simple, fast and the core code is unaffected. There is an unique constraint on the page field in PageAvatars model so you will not be able to add more than one avatar per page.
If there is something unclear in the code or you have a better idea I am ready to hear it.
