Is it ok to super/save() twice in model.save()?

1,893 views
Skip to first unread message

Richard Colley

unread,
May 13, 2010, 3:54:49 AM5/13/10
to Django users
I have overriden the save method on my model, and want to use the id
field (default autoincrementing pk) to set one of the other fields.

Is this ok:

class MyModel(models.Model):
...
def save(self, *args, **kwargs):
super(MyModel, self).save(*args, **kwargs)
self.some_string_field = "Some text with id=%d" % self.id
super(MyModel, self).save(*args, **kwargs)

It seems to work, but I'm confused about some side effects. If I use
obj, created = MyModel.get_or_create(...)
then created is always False, even when a new object is created. Is
that because of the 2 super.save() calls?

I guess I could pull this behaviour out of the model, and place into
the view that manipulates it, but I feel the functionality rightfully
belongs in the model.

Any help appreciated.

Thanks,
Richard

--
You received this message because you are subscribed to the Google Groups "Django users" group.
To post to this group, send email to django...@googlegroups.com.
To unsubscribe from this group, send email to django-users...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-users?hl=en.

zinckiwi

unread,
May 13, 2010, 8:58:21 AM5/13/10
to Django users
> I have overriden the save method on my model, and want to use the id
> field (default autoincrementing pk) to set one of the other fields.
>
> Is this ok:
>
>   class MyModel(models.Model):
>     ...
>     def save(self, *args, **kwargs):
>       super(MyModel, self).save(*args, **kwargs)
>       self.some_string_field = "Some text with id=%d" % self.id
>       super(MyModel, self).save(*args, **kwargs)
>
> It seems to work, but I'm confused about some side effects.  If I use
>   obj, created = MyModel.get_or_create(...)
> then created is always False, even when a new object is created.  Is
> that because of the 2 super.save() calls?

That seems a reasonable assumption, though I haven't looked at the
get_or_create() code.

When I've needed to do something similar (like using the pk of a model
as a folder name for file uploads, to keep different objects'
filenames from colliding) I've opted instead for creating a surrogate
key (in that example, a uuid).

Can you tell me a bit more about your use case? Obviously it must be
something a little more complex than simply storing a string that
contains the pk, or you would just have a method to calculate that
string. What exactly is it that requires access to the pk on create?

Regards
Scott

Richard Colley

unread,
May 13, 2010, 10:52:40 PM5/13/10
to Django users
Exactly as you had described ... I wanted to upload images to paths
with the model instance's id in it. Seemed easier for a human to
relate to that way (in case of maintenance outside of django).
However, the uuid approach or similar may be the way forward.

zinckiwi

unread,
May 14, 2010, 8:28:27 AM5/14/10
to Django users
Righto -- unfortunately that's the best solution I've come up with to
date. Here's the detailed version:

# models.py
def get_upload_path(instance, filename):
return "%s/%s/%s" % (instance.__class__.__name__.lower(),
instance.uuid, filename,)

def get_uuid():
import uuid
return str(uuid.uuid4())

class MyModel(models.Model):
...
pdf = models.FileField(max_length=200, upload_to=get_upload_path)
uuid = models.CharField(max_length=36, default=get_uuid,
editable=False)

That results in <model_name>/<uuid>/<original_file_name>.

I certainly did entertain a minimal-save-to-get-the-PK followed by the
"real" save, but didn't even get as far as discovering that it could
cause problems with shortcut functions. What turned me off at the
outset was needing to provide fake or default values for every other
field in the model, deleting that one if the "real" save failed, etc.
Seemed like a lot of busywork and prone to error.

Regards
Scott


On May 13, 10:52 pm, Richard Colley <richard.col...@gmail.com> wrote:
> Exactly as you had described ... I wanted to upload images to paths
> with the model instance's id in it.  Seemed easier for a human to
> relate to that way (in case of maintenance outside of django).
> However, the uuid approach or similar may be the way forward.

Richard Colley

unread,
May 14, 2010, 9:46:44 AM5/14/10
to Django users
Thanks Scott, that's more or less what we've finalised on. Appreciate
your feedback!

Richard
Reply all
Reply to author
Forward
0 new messages