Re: Overriding a Model's save() method does not allow proper creation of ManyToMany related objects
On Nov 15, 2007 3:13 PM, Gary Wilson <[EMAIL PROTECTED]> wrote: > > Collin Grady wrote: > > Are you seeing this behavior in admin? If so, I believe that is what is > > actually at fault, since it hard-sets m2ms, which would clear anything > > set in save(). > > I know Russ didn't take much of a liking to this idea last time I mentioned > it, but... Just to reiterate what I said last time: Firstly, this would be _huge_ backwards incompatibility. If this change were to be made, finding and fixing models that relied upon the old behaviour would be a non-trivial exercise. In the absence of a _very_ compelling reason to make this change, you'll find me coming down -1 for this reason alone. Secondly, although an m2m field is defined on a single model, isn't associated with a single model. It's associated with _two_ models. To me, this means that putting the save on the relation (i.e., when you assign the m2m attribute) rather than associating it with a particular model makes conceptual sense. If we were to move the m2m saving as part of the model save, you need to know which side of the m2m the field was defined upon, which is unwieldy because it presupposed knowledge of the model design. Alternatively, if you put the m2m saving on the save method of both models, there would be two different ways to effect the same database change. Thirdly, I'm not completely convinced that your proposed scheme won't end up with similar (or possibly even 0worse) race conditions. At present, the rules are simple - save all your models, then add all your m2m relations. If we moved saving m2m to the instances, then when you save instance X, you need to be sure that all the instances with which X has an m2m relation are also saved (say, instance Y). If instance Y then has an m2m relation back onto X through some other attribute, you've got yourself tied in a knot. > As far as AutomaticManipulator.save(), it appears that there is follow and > edit_inline logic mixed in with the manytomany saving. Does anyone who is > more familiar with newforms-admin know if the situation would be any better > there? I certainly hope so - that's why we're building newforms :-) Broadly, though, newforms has much better separation of concerns when it comes to dealing with inline objects; each inline object is handled as a discrete form, tied up in a formset wrapper that knows which object the formset is related to. This means there are a lot more (and a lot better isolated) points for customization of save behaviour. Yours, Russ Magee %-) --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: Overriding a Model's save() method does not allow proper creation of ManyToMany related objects
Collin Grady wrote:
> Are you seeing this behavior in admin? If so, I believe that is what is
> actually at fault, since it hard-sets m2ms, which would clear anything
> set in save().
I know Russ didn't take much of a liking to this idea last time I mentioned
it, but...
That's why I say this functionality shouldn't be part of the admin, but rather
built in to the manytomany field or the model. Let the manytomany field store
the state it needs to in order to know what needs to be saved, and have that
happen when you call the model instance's save().
So model instance save() would be something like:
def save(self, ...)
normal_saving_of_self()
for field in self.m2mfields:
# This could add or remove objects.
field.save_pending()
Dmitri's overridden save() would become:
def save(self):
regex = re.compile("\{\{([^\}]{2,60})\}\}")
words = regex.findall(self.body)
self.body = regex.sub("\\1",self.body)
for word in words:
if len(word.strip()) > 0:
try:
# This not actually saved do database yet, but stored
# in the keywords field.
self.keywords.create(value=word.strip().title())
except Exception, e:
print e
super(Chapter,self).save() # <-- saving of data and m2m data happens here
This allows manytomany fields to be treated like any of the other model fields
(even though it's a bit different in that the data is not stored in the same
table as the other fields).
As far as AutomaticManipulator.save(), it appears that there is follow and
edit_inline logic mixed in with the manytomany saving. Does anyone who is
more familiar with newforms-admin know if the situation would be any better
there?
Gary
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---
Re: Overriding a Model's save() method does not allow proper creation of ManyToMany related objects
Interesting, I'll have a look at it after this project is done hehe. I totally understand and am simply throwing ideas around. However it seems that things are already moving in that direction. :) Thanks for the link. //D On 11/7/07, Joseph Kocherhans <[EMAIL PROTECTED]> wrote: > > > On 11/7/07, Dmitri Fedortchenko <[EMAIL PROTECTED]> wrote: > > > > The Admin class should be able to define post_ and pre_save hooks that > are > > called before or after all Manipulation of the model is done (there are > > post_save hooks for the save method, but they are still called before > the > > Manipulator is done working with the model) > > Something similar is in progress already. > http://code.djangoproject.com/wiki/NewformsAdminBranch > > It is however, mostly undocumented and subject to change. These things > take time :) > > Joseph > > > > --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: Overriding a Model's save() method does not allow proper creation of ManyToMany related objects
On 11/7/07, Dmitri Fedortchenko <[EMAIL PROTECTED]> wrote: > > The Admin class should be able to define post_ and pre_save hooks that are > called before or after all Manipulation of the model is done (there are > post_save hooks for the save method, but they are still called before the > Manipulator is done working with the model) Something similar is in progress already. http://code.djangoproject.com/wiki/NewformsAdminBranch It is however, mostly undocumented and subject to change. These things take time :) Joseph --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: Overriding a Model's save() method does not allow proper creation of ManyToMany related objects
Perhaps. However I believe that customization of the admin is not a bad thing. On the contrary it allows people to use it for most everything that they need without having to write duplicate code... One such thing is the thread_locals hack for created_by/updated_by... Imagine if you wanted to dynamically set the created_by and updated_by fields and were forced to "write a custom view" for every model you have... that basically takes away the usefulness of the admin. Here is an idea: The Admin class should be able to define post_ and pre_save hooks that are called before or after all Manipulation of the model is done (there are post_save hooks for the save method, but they are still called before the Manipulator is done working with the model) On 11/7/07, Collin Grady <[EMAIL PROTECTED]> wrote: > > > Dmitri Fedortchenko said the following: > > Well django does recommend using the admin for everything admin-related, > > if I stop using the admin for editing an article, then I would have to > > write a whole bunch of code just to edit a single article. Suddenly the > > benefit of a unified admin interface is lost and I have to deal with > > permissions, UI, validation, forms.. it seems like a nightmare for > > something should work in the first place... ( i.e. overriding the save() > > method to add extra functionality) > > > > From the django features list: > > A dynamic admin interface: it's not just scaffolding it's the whole > house > > [The philosophy here is that your site is edited by a staff, or a > > client, or maybe just you and you don't want to have to deal with > > creating backend interfaces just to manage content.] > > > > The message seems clear to me. ;) > > It's not meant to do absolutely everything though - trying to hack it to > do something it doesn't is usually harder than writing a really simple > custom view :) > > -- > Collin Grady > > If I had any humility I would be perfect. > -- Ted Turner > > > > > --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: Overriding a Model's save() method does not allow proper creation of ManyToMany related objects
Dmitri Fedortchenko said the following: > Well django does recommend using the admin for everything admin-related, > if I stop using the admin for editing an article, then I would have to > write a whole bunch of code just to edit a single article. Suddenly the > benefit of a unified admin interface is lost and I have to deal with > permissions, UI, validation, forms.. it seems like a nightmare for > something should work in the first place... ( i.e. overriding the save() > method to add extra functionality) > > From the django features list: > A dynamic admin interface: it's not just scaffolding it's the whole house > [The philosophy here is that your site is edited by a staff, or a > client, or maybe just you and you don't want to have to deal with > creating backend interfaces just to manage content.] > > The message seems clear to me. ;) It's not meant to do absolutely everything though - trying to hack it to do something it doesn't is usually harder than writing a really simple custom view :) -- Collin Grady If I had any humility I would be perfect. -- Ted Turner --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: Overriding a Model's save() method does not allow proper creation of ManyToMany related objects
Well django does recommend using the admin for everything admin-related, if I stop using the admin for editing an article, then I would have to write a whole bunch of code just to edit a single article. Suddenly the benefit of a unified admin interface is lost and I have to deal with permissions, UI, validation, forms.. it seems like a nightmare for something should work in the first place... (i.e. overriding the save() method to add extra functionality) From the django features list: A dynamic admin interface: it's not just scaffolding it's the whole house [The philosophy here is that your site is edited by a staff, or a client, or maybe just you and you don't want to have to deal with creating backend interfaces just to manage content.] The message seems clear to me. ;) //D On 11/7/07, Collin Grady <[EMAIL PROTECTED]> wrote: > > > Dmitri Fedortchenko said the following: > > Still, the most logical course is to override the save method. > > I'd say the more logical course is to stop using admin for this ;) > > -- > Collin Grady > > QOTD: > "If I'm what I eat, I'm a chocolate chip cookie." > > > > --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: Overriding a Model's save() method does not allow proper creation of ManyToMany related objects
Dmitri Fedortchenko said the following: > Still, the most logical course is to override the save method. I'd say the more logical course is to stop using admin for this ;) -- Collin Grady QOTD: "If I'm what I eat, I'm a chocolate chip cookie." --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: Overriding a Model's save() method does not allow proper creation of ManyToMany related objects
That is what I thought as well, however I checked, and the _add_items() method is only called once with the correct objects (meaning the existing keywords and the keywords that I added). Are you telling me that the admin actually has it's own method for setting the relationships that is outside the django.models.fields.related module? //Dmitri On Nov 7, 2:02 am, Collin Grady <[EMAIL PROTECTED]> wrote: > Are you seeing this behavior in admin? If so, I believe that is what is > actually at fault, since it hard-sets m2ms, which would clear anything > set in save(). > > Also, you don't need to call save() again after adding to an m2m, since > it does not edit the model instance itself, but the related table. > > -- > Collin Grady > > Haste makes waste. > -- John Heywood --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "Django developers" group. To post to this group, send email to django-developers@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/django-developers?hl=en -~--~~~~--~~--~--~---
Re: Overriding a Model's save() method does not allow proper creation of ManyToMany related objects
Hi
Basically I have followed the call all the way to the execution of the
cursor.execute() on line 343 in models.fields.related.
I have not actually delved into the transaction method. However in my
setup I am using the django admin, I have not set up any automatic
transaction handling and the save method does is not transaction
managed. Also I am running this on an existing object, and even if I
was not, the primary key would have been created since the save method
is called before I add new m2ms.
Calling the super(Chapter,self).save() AFTER adding new m2ms is not
needed, nor does it work.
I also checked to make sure that the
django.models.fields.related._remove_items() method is not called
after the _add_items() method, to restore some previous state.
My only conclusion is that somehow the transaction of the save()
cancels out the transaction of the _add_items() method...
Maybe it's the
save.alters_data = True
flag?
or something to do with the dispatcher? I have not yet gone that deep
into the source...
But I can assure you that I am using as basic a setup as possible.
It is simply django admin, MySQL with InnoDB and that very save method
that you see in my initial post.
So repeating this should not be a problem...
I'll try to delve deeper into the transaction side of things, but any
insight you can give me would be appreciated.
//Dmitri
On Nov 7, 2:25 am, Malcolm Tredinnick <[EMAIL PROTECTED]>
wrote:
> On Wed, 2007-11-07 at 00:50 +, Dmitri Fedortchenko wrote:
> > Example code:
>
> > def save(self):
> >regex = re.compile("\{\{([^\}]{2,60})\}\}")
> >words = regex.findall(self.body)
> >self.body = regex.sub("\\1",self.body)
> >super(Chapter,self).save()
> >for word in words:
> >if len(word.strip()) > 0:
> >try:
> >self.keywords.create(value=word.strip().title())
> >except Exception, e:
> >print e
> >print self.keywords.all() # This prints the correct keywords!
> >super(Chapter,self).save()
>
> > The outcome of this code is that new Keywords are created, but they
> > are not bound to this Chapter.
> > Simply calling chapter.keywords.create(value="Test") will indeed
> > create a new keyword with the value "Test" bound to the chapter. I am
> > running this from the django admin btw.
>
> Can you explain what is meant to be going on here?
>
> If you are trying to save m2m relations for an object that has not been
> saved yet, you're out of luck. We need to know the current object's pk
> value before it can be used in a m2m table and that isn't always
> available (with auto primary keys).
>
> Normally, if you want to adjust the m2m relations for a model, hooking
> into the post_save signal is the way to go.
>
>
>
> > The problem seems to be in the
> > django.db.models.fields.related._add_items method, somewhere around
> > line 340.
> > The fact is that the insertion query is executed, but for some reason
> > it is not committed, perhaps it is a transaction issue?
>
> Perhaps it is. Perhaps it isn't. How have you established the query was
> executed? What does the commit_unless_managed() call end up doing in
> your case? It might depend on your particular setup, too. Do some more
> debugging and see what shows up if you think there's a problem there.
> However, do keep in mind that it's generally impossible to do everything
> with m2m's inside a model's save method because of the chicken-and-egg
> problem with primary keys noted above.
>
> Regards,
> Malcolm
>
> --
> Tolkien is hobbit-forming.http://www.pointy-stick.com/blog/
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---
Re: Overriding a Model's save() method does not allow proper creation of ManyToMany related objects
On Wed, 2007-11-07 at 00:50 +, Dmitri Fedortchenko wrote:
> Example code:
>
> def save(self):
> regex = re.compile("\{\{([^\}]{2,60})\}\}")
> words = regex.findall(self.body)
> self.body = regex.sub("\\1",self.body)
> super(Chapter,self).save()
> for word in words:
> if len(word.strip()) > 0:
> try:
> self.keywords.create(value=word.strip().title())
> except Exception, e:
> print e
> print self.keywords.all() # This prints the correct keywords!
> super(Chapter,self).save()
>
>
> The outcome of this code is that new Keywords are created, but they
> are not bound to this Chapter.
> Simply calling chapter.keywords.create(value="Test") will indeed
> create a new keyword with the value "Test" bound to the chapter. I am
> running this from the django admin btw.
Can you explain what is meant to be going on here?
If you are trying to save m2m relations for an object that has not been
saved yet, you're out of luck. We need to know the current object's pk
value before it can be used in a m2m table and that isn't always
available (with auto primary keys).
Normally, if you want to adjust the m2m relations for a model, hooking
into the post_save signal is the way to go.
>
> The problem seems to be in the
> django.db.models.fields.related._add_items method, somewhere around
> line 340.
> The fact is that the insertion query is executed, but for some reason
> it is not committed, perhaps it is a transaction issue?
Perhaps it is. Perhaps it isn't. How have you established the query was
executed? What does the commit_unless_managed() call end up doing in
your case? It might depend on your particular setup, too. Do some more
debugging and see what shows up if you think there's a problem there.
However, do keep in mind that it's generally impossible to do everything
with m2m's inside a model's save method because of the chicken-and-egg
problem with primary keys noted above.
Regards,
Malcolm
--
Tolkien is hobbit-forming.
http://www.pointy-stick.com/blog/
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---
Overriding a Model's save() method does not allow proper creation of ManyToMany related objects
Example code:
def save(self):
regex = re.compile("\{\{([^\}]{2,60})\}\}")
words = regex.findall(self.body)
self.body = regex.sub("\\1",self.body)
super(Chapter,self).save()
for word in words:
if len(word.strip()) > 0:
try:
self.keywords.create(value=word.strip().title())
except Exception, e:
print e
print self.keywords.all() # This prints the correct keywords!
super(Chapter,self).save()
The outcome of this code is that new Keywords are created, but they
are not bound to this Chapter.
Simply calling chapter.keywords.create(value="Test") will indeed
create a new keyword with the value "Test" bound to the chapter. I am
running this from the django admin btw.
The problem seems to be in the
django.db.models.fields.related._add_items method, somewhere around
line 340.
The fact is that the insertion query is executed, but for some reason
it is not committed, perhaps it is a transaction issue?
It is simply broken when called from within the save() method of a
Model instance...
I haven't started a ticket because I am not sure if this is bug or a
feature ;-)
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---
