* cc: kmike84@… (added)
* ui_ux: => 0
* easy: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:12>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Comment (by anonymous):
I had more luck with:
{{{
def refresh_from_db(self):
"""Refreshes this instance from db
https://code.djangoproject.com/ticket/901
"""
from_db = self.__class__.objects.get(pk=self.pk)
for field in self.__class__._meta.fields:
setattr(self, field.name, getattr(from_db, field.name))
}}}
But I don't know enough to know if it's correct, only that it seems to
work.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:13>
* status: closed => new
* resolution: wontfix =>
Comment:
Shouldn't objects be reloaded from the database by default, after calling
the save() method? Say you have stored procedures, triggers or rules
modifying the data. Django expects itself to be the only data-modifying
user to the database, ever, but in many cases this assumption does not
hold true.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:14>
* status: new => closed
* resolution: => wontfix
Comment:
For the fourth time in this thread: *please* do not reopen tickets closed
wontfix by a core developer. If you'd like to advocate for this feature,
do so on the mailing list, and if consensus is reached _then_ the ticket
can be reopened.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:15>
Comment (by anonymous):
Replying to [comment:13 anonymous]:
> I had more luck with:
> {{{
> def refresh_from_db(self):
> """Refreshes this instance from db
> https://code.djangoproject.com/ticket/901
> """
> from_db = self.__class__.objects.get(pk=self.pk)
> for field in self.__class__._meta.fields:
> setattr(self, field.name, getattr(from_db, field.name))
> }}}
> But I don't know enough to know if it's correct, only that it seems to
work.
If we're not going to get a method to do this for us, it would be nice if
one of the core developers could bless or provide a code sample to update
'self' with the latest data from the db instance.
I've seen this one, as well as one that updates self.__dict__ :
http://stackoverflow.com/questions/4377861/reload-django-object-from-
database
Which method can we depend upon still working in the future?
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:16>
Comment (by anonymous):
So I just came across the presentation "Why Django Sucks and How we Can
Fix it" at http://blip.tv/djangocon/why-django-sucks-and-how-we-can-fix-
it-4131303
As I watched it, this bug immediately came to mind. I feel it is a
perfect example of exactly what Eric was talking about in his talk.
There are multiple StackOverflow questions asking how to do this, there
are several answers there on how to do this, none of which are ensured to
keep future compatibility. Some are stated as incomplete ("Oh, this
won't work if you have foreign keys"), and potentially have other quirks.
And there is no documented way that suggests the best/supported way to do
this.
There's a suggested patch above.
Note: The use case for this request isn't:
obj = Foo.objects.get(pk=1)
<some external side effects>
obj = obj.reload() # i.e., obj = Foo.objects.get(pk=obj.id)
It is people wanting to do a reload from *within* the model. And "self =
self.objects.get(pk=self.id)" isn't going to do it, obviously!
The use case is more like:
class Foo(Model):
def blah():
# maybe we need to use .update() for some reason, like
# atomicity. i.e., only update iff x is 0 as a atomicity
# check.
Foo.objects.filter(pk=self.id, x=0).update(x=1, y=10)
<maybe some other side effects>
# make sure I have the latest attributes
self.reload()
...
I don't want the client of the Model Foo to have to know "Oh, well, I need
to refresh my copy of the object after I call blah()". And other
references to the object that called blah() will still be wrong.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:17>
Comment (by simon29):
Eric makes an important point. I gave up a long time ago trying to make
simple feature suggestions like these, no matter how big/small (6yrs ago
by the look of this thread).
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:18>
Comment (by anonymous):
Funny, we've also come to the conclusion that making Django feature
requests is a waste of time.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:19>
Comment (by russellm):
Making feature requests is in no way a waste of time. Every version of
Django has had features that the previous version didn't have. Those
features were requested by *someone*. So it is demonstrably *not* futile.
However, expecting that merely *making* a feature request will in some way
magically result in your feature being added? That *is* futile.
The difference between the features that *have* been added, and the
features that *haven't* been added, is that someone actually did the work.
And I don't *just* mean writing a patch, either. I mean someone engaged
with the community. Had a discussion. Resolved any issues or concerns that
existed.
Let's take this exact ticket as an example. If you actually *read* the
discussion on this ticket, members of the core team have expressed
concerns about the idea, and have said on *three separate occasions* what
needs to happen in order for this issue to move forward: *Raise the issue
on Django-developers*.
So, if this is such an important topic, such an essential feature, the
proponents of this idea *must* have raised it on django-developers, right?
Well, no. As far as I can make out, it's been raised exactly once, in
2008, and after a single post from Simon, and a single response from
Malcolm, the conversation died. Ticket #901 came up in passing in 2009 on
a django-users post. So not only hasn't there been a vibrant developer
debate, there's hasn't been a flood of requests for the feature on django-
users, either.
Suggesting features is only futile if you are labouring under the illusion
that the core team is sitting here waiting for ideas of things to do. Let
me shatter that illusion for you. We have plenty of things to do. What we
don't have, and what we want, is people offering to help out.
If you want it, you have to make it happen.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:20>
Comment (by simon29):
I think it's important we as a community *acknowledge* there is an issue
here. Eric, myself, and plenty of others seem to think so. It's not
unfixable either.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:21>
Comment (by aaugustin):
Yes, it's all about the community, and the community is on the django-
developers mailing list, not here.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:22>
Comment (by akaariai):
FWIW I think this feature is a good idea - but the mailing list is the
right place to discuss this.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:23>
Comment (by anonymous):
Omg, huge +1 here. Whenever I use model forms I have to query for objects
again if I don't save them (i.e., form data is not valid), which wastes a
query.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:24>
Comment (by wim@…):
Hi people,
Thanks for taking the time to respond to this ticket. A question has been
raised on the Django mailing list if we can reconsider re-opening it. The
mailing thread is here:
https://groups.google.com/forum/?fromgroups=#!topic/django-
developers/Dcqq7zr2VZo
In addition, if you want to discuss the responsiveness of django community
in general or on this ticket, you can do so here:
https://groups.google.com/forum/?fromgroups=#!topic/django-
developers/DUQtBrM2iTs
I am very sorry to hear that people feel unfriendly received and not
appreciated. For myself, since Luke Plant gave me a long and detailed
answer on just a question I had, and which I put on the mailing list, I
have been feeling very welcome here.
Thank you all for your time spent in this community.
Wim
PS For the record, in a Google Group you can become a member of a mailing
list and read all the mail online, while no mail is being sent to you.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:25>
* status: closed => new
* type: enhancement => New feature
* component: Core (Other) => Database layer (models, ORM)
* version: => master
* has_patch: 1 => 0
* resolution: wontfix =>
* stage: Design decision needed => Accepted
Comment:
The discussion on django-developers concluded that this ticket should be
reopened, and it contains a concrete proposal.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:26>
* cc: aarongc@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:27>
* cc: james@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:28>
* needs_better_patch: 0 => 1
* has_patch: 0 => 1
Comment:
A simple first implementation:
https://github.com/planop/django/tree/ticket_901 based on the ideas in the
django-developers thread.
Adds modelinstance.refresh([*fieldnames]) method to reload (part of) the
object from the database.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:29>
* cc: un33kvu@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:30>
Comment (by me@…):
Looking at the discussion on the django-developers mailing list, several
developers have expressed concern about messy corner cases.
Koen: you've set 'patch needs improvement'. What issues are remaining with
the patch?
Looking at this issue, I think adding a Model.fresh_copy method would be
sufficient for many use cases and would be straightforward to implement.
Certainly all my use cases would be satisfied. I'm primarily interested in
unit tests, where I've made several requests and I want to verify the
state of the database.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:31>
Comment (by me@…):
I've opened a pull request: https://github.com/django/django/pull/2194
that defines a Model.get_again method. I think this is a good compromise.
I'd love feedback.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:32>
Comment (by timo):
You can find [https://groups.google.com/d/msg/django-
developers/Dcqq7zr2VZo/Z-JkdZ0y2qkJ the concrete proposal] that was agreed
to on the mailing list. The latest pull request doesn't address the
problem this way.
The patch from Koen looks more on the right track, but I haven't reviewed
it in detail.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:33>
* cc: mike@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:34>
* owner: adrian => akaariai
* status: new => assigned
Comment:
Now that Model.from_db() has been added I think this will be important
addition - the most important use case being deferred field loading and
altering the loading behavior in that case. Currently for example
implementing save only modified fields feature will be impossible to do
for deferred fields.
I am going to go with the concrete API proposal mentioned above
{{{
Model.refresh() reloads all non-deferred local
field values (that is, all fields in the current model which have a
database column). In addition refresh() will make sure that cached
values dependent of the reloaded values will be cleared. That is, if
you do foo.refresh(), then when foo.user_id is reloaded foo.user will
be cleared (assuming the reloaded user_id is different from the
original value). This is to ensure that next access to foo.user will
reload the related object, too.
The refresh() method makes sure descriptors are honored, for example
custom field's to_python() method will be called for refreshed values.
The refresh() method accepts *args, so that one can specify which
fields to reload. This is useful so that deferred attribute loading
can use refresh(), and by overriding refresh() it will be possible to
customize how deferred loading happens.
Deferred field loading is altered to use .refresh().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:35>
* needs_docs: 0 => 1
Comment:
I implemented the above design plan in PR
https://github.com/django/django/pull/2882.
The changes to the plan are:
- the method name is refresh_from_db()
- no *args, instead kwarg fields containing an iterable of fields to
refresh
- added using kwarg, by which one can alter the database to refresh from
In addition Model.get_deferred_fields() was added. This is needed in
public API so that it is possible to create a refresh_from_db() method
that reloads all deferred fields when one of those is accessed.
As discussed on mailing list, the reloading doesn't affect non-local
values. For example, if you have done prefetching, the prefetched values
will not be invalidated. It seems impossible to code a refresh_from_db()
method that actually clears all database dependent value from the model,
for the simple reason that in many cases Django can't even know what
database dependent values are.
I wonder if we need better handling for o2o fields. This has two cases:
1) o2o defined on refreshed model - when doing refresh, should we also
clear the cached value on the pointed model?
2) o2o defined on remote model - when doing refresh, should we make sure
that the cached value on refreshed model is still valid?
So, with models:
{{{
class M1:
pass
class M2:
m1 = O2OField(M1)
}}}
case 2)'s question is if we load instance m1_1 <-> m2_1 from db, and then
m2_1.o2o is changed to point to m1_2, when doing m1_1.refresh() should we
notice that m1_1.m2 has changed to None? In case 1), if we refresh m2_1()
we do set m1 to None. But should we also set m1_1.m2 to None, too?
Both of these are getting to "clear all database defined state on the
model" category. Still, I am not sure if we should do at least 1). If 1)
is implemented, then it is possible to force refresh of the reverse o2o by
doing m1_1.m2.refersh_from_db().
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:36>
* cc: mail@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:37>
* needs_better_patch: 1 => 0
* needs_docs: 1 => 0
Comment:
I think this is now ready for checkin, but as this is a non-trivial new
feature I'd like to get this reviewed before marking ready for checkin.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:38>
* owner: akaariai => timgraham
* stage: Accepted => Ready for checkin
Comment:
Will cleanup docs/docstrings a bit before committing.
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:39>
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"c7175fcdfe94be60c04f3b1ceb6d0b2def2b6f09"]:
{{{
#!CommitTicketReference repository=""
revision="c7175fcdfe94be60c04f3b1ceb6d0b2def2b6f09"
Fixed #901 -- Added Model.refresh_from_db() method
Thanks to github aliases dbrgn, carljm, slurms, dfunckt, and timgraham
for reviews.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/901#comment:40>