Re: Proposal: Prevent data loss in the admin

[Steven Cummings]

FWIW, this was on my mind a few years ago and I created this [1] ticket. It
spidered out into one or two more, but the basic ideas were:

* optimistic locking could be implemented by enhancing saves with
conditions (i.e., save_if)
* a row-modified count could help that function also tell you if the save
condition passed or failed
* rows modified *may* be used now to update data via QuerySet.update, but
allegedly not all of the DB implementations provide the count in the same
way (e.g., matched vs. updated)
* the modified count discrepancy was worth spinning up another ticket to
discuss and investigate

I agree with Russell, I think the need for this is much broader than the
admin.

[1] https://code.djangoproject.com/ticket/16549

On Mon Dec 08 2014 at 7:10:28 PM Russell Keith-Magee <
russ...@keith-magee.com> wrote:

> Hi Rune,
>
> I agree with Tim. This is clearly a problem that exists, and I agree it
> would be good to fix it. However, I wouldn't want to see this as an "Admin
> only" fix.
>
> I'd rather see a generic hook that Admin then leverages as the "first
> customer". A similar approach was taken with object-level permissions - a
> generic framework for object-level permissions was added, and admin was
> modified to support that API as part of the proof that the API was
> sufficient.
>
> From an implementation perspective, I also have a mild negative reaction
> to the idea of using a separate table in the database for locks. Using a
> database row to marshal this behavior sounds like adding load to a database
> when all you need is a transient marker. A backend API that lets you use
> memcache, redis, or even a file-based backend might be called for. And
> that's assuming we take the "separate lock table" approach - I can think of
> other approaches that involve version indicators on the model itself. These
> are just initial impressions, however; I haven't given the problem a whole
> lot of thought, so these might not be viable ideas given a little more
> consideration.
>
> Yours,
> Russ Magee %-)
>
>
> On Mon, Dec 8, 2014 at 11:37 PM, Tim Graham  wrote:
>
>> Hi Rune,
>>
>> It's not clear to me that a fix that's coupled to the admin is the way to
>> go when this problem could exist for any model form, etc.
>>
>> One ticket you didn't mention that seems like it could be a good place to
>> start is https://code.djangoproject.com/ticket/16549
>>
>> Tim
>>
>> On Monday, December 8, 2014 8:04:30 AM UTC-5, Rune Kaagaard wrote:
>>>
>>> @Daniele
>>>
>>> I struggled a bit with how to display the error message, and your
>>> solution makes perfect sense.
>>>
>>> Best,
>>> Rune Kaagaard
>>>
>>  --
>> You received this message because you are subscribed to the Google Groups
>> "Django developers (Contributions to Django itself)" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to django-developers+unsubscr...@googlegroups.com.
>> To post to this group, send email to django-developers@googlegroups.com.
>> Visit this group at http://groups.google.com/group/django-developers.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/django-developers/f1a56260-89d7-4439-aed1-9407d799f62f%40googlegroups.com
>> 
>> .
>>
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>  --
> You received this message because you are subscribed to the Google Groups
> "Django developers (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To post to this group, send email to django-developers@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/CAJxq84_Kz_6ZCTzxEjaZm61hdfYcKnQNHw-xyPr%3DzX%3Dtc8GYvw%40mail.gmail.com
> 
> .
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To post to this group, send email to django-developers@googlegroups.com.
Visit this group at http://groups.google.com/group/django-developers.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAODHHFAJct5FCDD8gcrB0_L-yOwXuyMx6PzmeHzHacyHgVhC9w%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: QuerySet refactoring

[Steven Cummings]

That answers my question. Thanks! Perhaps I'll try to make
matched-vs-updated a point of discussion for Django 2.0. As for the other
stuff, I'll give consideration to work on save_base and maybe work up a
proposal there.
--
Steven


On Tue, Jun 26, 2012 at 2:31 AM, Anssi Kääriäinen
<anssi.kaariai...@thl.fi>wrote:

> On 26 kesä, 08:41, Steven Cummings <estebis...@gmail.com> wrote:
> > Sorry for chiming in so late. I very much like the goals of this effort,
> > particularly bringing clarity to some of the internal APIs. Some related
> > points for your consideration:
>
> No problem. I have been so busy lately that I haven't gotten a single
> commit done... Things look a bit calmer, so I hope I will be able to
> start working on Django again.
>
> > * Would the rows matched vs. updated issue be resolved or clarified in
> this
> > effort [1]?
>
> I don't believe this one can be changed. Django currently returns rows
> matched, and to remain backwards compatible a flag to .update() would
> be needed. The flag would change the return value from matched to
> changed rows. This doesn't feel right.
>
> > * It seems like the work I had started to expose accurate update/delete
> > counts [2], and further provide the capability for conditional updates
> and
> > deletes [3] would be more clearly done on the basis of such a refactor.
> > Does that seem accurate to you or will it not make much of a difference
> > there?
>
> For the optimistic concurrency control for model .save(), it seems
> that splitting the save_base() to smaller parts could allow subclasses
> to do whatever needed for good optimistic concurrency control. Another
> option is some sort of hook which allows the instance to add
> additional conditions to the save.
>
> I must note that the idea of the refactor is not to do any drastic
> rewrites. The structure of the ORM will stay mostly the same.
> Currently my aim is at the add_q/add_filter stage of the ORM, and I
> don't see much cross section with what you are doing at this stage. I
> don't have any long term plans of where this all is going, it is just
> cleanup/bugfixing of existing code. Most likely the save_base() will
> need some refactoring too.
>
>  - Anssi
>
> --
> 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
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: QuerySet refactoring

[Steven Cummings]

Sorry for chiming in so late. I very much like the goals of this effort,
particularly bringing clarity to some of the internal APIs. Some related
points for your consideration:

* Would the rows matched vs. updated issue be resolved or clarified in this
effort [1]?
* It seems like the work I had started to expose accurate update/delete
counts [2], and further provide the capability for conditional updates and
deletes [3] would be more clearly done on the basis of such a refactor.
Does that seem accurate to you or will it not make much of a difference
there?

Thanks.

--
Steven

[1] https://code.djangoproject.com/ticket/17435
[2] https://code.djangoproject.com/ticket/16891
[3] https://code.djangoproject.com/ticket/16549

On Thu, Jun 14, 2012 at 3:58 AM, Anssi Kääriäinen
wrote:

> On 14 kesä, 10:59, Aymeric Augustin
>  wrote:
> > Hello Anssi,
> >
> > I'm familiar with the topic since I tried to review some of your
> > refactoring patches (before you gained the ability to commit them
> > yourself).
> >
> > I'm convinced that this refactoring is useful, because it is likely to
> > fix some bugs, especially in features that were added to the ORM long
> > after its original design, and it will make ongoing maintenance
> > easier.
> >
> > It's also very difficult to review and I'm afraid I may not be able to
> > help a lot, besides sanity-checking patches. Anyway, ask loudly for
> > reviews when you have patches ready, and I'll try to join the effort.
>
> Thanks for all for the offers for reviews.
>
> I think I will go forward with the following plan:
>  - For small issues I will create a ticket + pull request and
> announce that I am going to commit the request. I will wait for couple
> of days and commit if no objections or requests for more time are
> raised.
>  - For medium issues I will ask for a review and wait for one.
>  - For bigger issues (db schemas, deepcopy() vs. clone()) there need
> to be some form of consensus that the approach is in fact correct.
>
> Lets see how the above works. If it doesn't, then lets figure out some
> other way. A branch for "orm-next" could work also...
>
> I will post a short request here on django-developers, and then detail
> the request in the ticket.
>
> For now the big issue needing review is the django.utils.tree/add_q
> refactor. https://code.djangoproject.com/ticket/17000#comment:7 and
> https://github.com/akaariai/django/commits/refactor_utils_tree
>
> I would like to get the qs deepcopy/clone() thingy moved forward or
> closed as wontfix. Luckily this is a well separated issue from rest of
> queryset refactor, so there is no hurry for this one:
> https://code.djangoproject.com/ticket/16759#comment:33
>
>  - Anssi
>
> --
> 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
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Django's CVB - Roadmap?

[Steven Cummings]

The docs around the provided CBVs and mixins are an issue, but also what is
missing is that programmers seem to still think those that are provided
should work for more cases than they probably do. When the narrative for
CBVs is documented, should Django focus on mixins and state the expected
limits of the provided classes? I think people are finding those limits
pretty quickly (e.g., "I need more than one form on this page!"), and then
perhaps conclude that CBV's are "broken". Perhaps the idea that your site
or Django app may need more specific base-classes needs more promotion?

--
Steven


On Fri, Jun 1, 2012 at 12:21 PM, Adam "Cezar" Jenkins <
emperorce...@gmail.com> wrote:

> On Fri, Jun 1, 2012 at 12:04 PM, Michael  wrote:
>
>> I am not ready to judge right now wether they are a good idea or not.
>>
>> I can completely agree that documentation makes the views more difficult
>> then they actually are. Every time I create a new view, I find myself going
>> to the source. I think this is a case where the learning curve initially is
>> a little steeper so better examples and best-practices will really help
>> with people's implementation and also this discussion.
>>
>>
> This is the real issue. The docs. I'm trying to use CBVs exclusively in my
> latest project. I end up in the source most of the time. It might be good
> to start people out with very basic CBVs and make the docs much more of a
> tutorial.
>
>
>
>> I have been wanting to write up documentation every time I go to the
>> source, but sadly haven't had the time. Maybe I will soon.
>>
>>
>>
>>
>>
>> On Fri, Jun 1, 2012 at 12:35 PM, Donald Stufft 
>> wrote:
>>
>>>  I tend to agree, in general, with the reply that there should be a
>>> function based api
>>> to cover the 80% use case, but in the case of Django's CBV's this is
>>> likely
>>> covered by the as_view method.
>>>
>>> On Friday, June 1, 2012 at 10:54 AM, Jacob Kaplan-Moss wrote:
>>>
>>> On Fri, Jun 1, 2012 at 10:14 AM, Victor Hooi 
>>> wrote:
>>>
>>> The reason for my post - is there anything from the Django core as a
>>> whole
>>> on this? What's the future roadmap like in terms of CBV's? Are there
>>> going
>>> to be any changes to it, or are we safe to assume things will be as they
>>> are
>>> now.
>>>
>>>
>>> I think as a whole we're divided. Luke's opinions differ from
>>> mine, and it's entirely unclear to me who's "right" or even if there's
>>> a "right" to be had. And that's just the two of us!
>>>
>>> I think this might be a situation where we need some more feedback
>>> from the community and some more time to decide what the right move
>>> here is.
>>>
>>> So... what do *you* think?
>>>
>>> Jacob
>>>
>>> --
>>> 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
>>> django-developers+unsubscr...@googlegroups.com.
>>> For more options, visit this group at
>>> http://groups.google.com/group/django-developers?hl=en.
>>>
>>>
>>>  --
>>> 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
>>> django-developers+unsubscr...@googlegroups.com.
>>> For more options, visit this group at
>>> http://groups.google.com/group/django-developers?hl=en.
>>>
>>
>>  --
>> 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
>> django-developers+unsubscr...@googlegroups.com.
>> For more options, visit this group at
>> http://groups.google.com/group/django-developers?hl=en.
>>
>
>  --
> 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
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: New feature: HTTP OPTIONS and improved HEAD for View

[Steven Cummings]

Created a ticket with the patch: https://code.djangoproject.com/ticket/17449
--
Steven


On Sun, Dec 11, 2011 at 3:29 PM, Steven Cummings <estebis...@gmail.com>wrote:

> I'll probably go ahead and log it and start the patch soon. Your feedback
> on truncation for HTTP HEAD was helpful, and the rest doesn't seem very
> controversial.
> --
> Steven
>
>
>
> On Sat, Dec 10, 2011 at 5:26 AM, Jamie Matthews 
> <jamie.matth...@gmail.com>wrote:
>
>> Just bumping this - can anyone suggest what the next steps should be?
>> It'd be nice to get it fixed for 1.4.
>>
>> Thanks,
>>
>> Jamie
>>
>> On Nov 28, 5:59 pm, Jamie Matthews <jamie.matth...@gmail.com> wrote:
>> > > Yeah, that implementation seems preferable. What was the reason for
>> backing
>> > > it out?
>> >
>> > Not sure. What would be the procedure for getting this changed? Open a
>> > new ticket, I assume?
>>
>> --
>> 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
>> django-developers+unsubscr...@googlegroups.com.
>> For more options, visit this group at
>> http://groups.google.com/group/django-developers?hl=en.
>>
>>
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: New feature: HTTP OPTIONS and improved HEAD for View

[Steven Cummings]

I'll probably go ahead and log it and start the patch soon. Your feedback
on truncation for HTTP HEAD was helpful, and the rest doesn't seem very
controversial.
--
Steven


On Sat, Dec 10, 2011 at 5:26 AM, Jamie Matthews wrote:

> Just bumping this - can anyone suggest what the next steps should be?
> It'd be nice to get it fixed for 1.4.
>
> Thanks,
>
> Jamie
>
> On Nov 28, 5:59 pm, Jamie Matthews  wrote:
> > > Yeah, that implementation seems preferable. What was the reason for
> backing
> > > it out?
> >
> > Not sure. What would be the procedure for getting this changed? Open a
> > new ticket, I assume?
>
> --
> 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
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: New feature: HTTP OPTIONS and improved HEAD for View

[Steven Cummings]

I assumed I needed the votes here to get going on that. In the past when
I've started with a ticket I've been directed here to gather support first.
--
Steven


On Mon, Nov 28, 2011 at 11:59 AM, Jamie Matthews
wrote:

> > Yeah, that implementation seems preferable. What was the reason for
> backing
> > it out?
>
> Not sure. What would be the procedure for getting this changed? Open a
> new ticket, I assume?
>
> --
> 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
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: New feature: HTTP OPTIONS and improved HEAD for View

[Steven Cummings]

On Sun, Nov 27, 2011 at 4:17 PM, Jamie Matthews wrote:

> > 2. Improve the default HTTP HEAD handler by ensuring it has a blank body,
> > as this is the expectation of a response to a HEAD request. Also, there
> > should be consideration for the GET handler not existing, which should
> > result in a 405. The code appears to assume it exists, which would result
> > in an AttributeError instead.
>
> For previous discussion on this, see this ticket:
> https://code.djangoproject.com/ticket/15668
>
> * Graham Dumpleton disagrees with your first point: "Python web
> applications should never themselves discard the response content for
> HEAD requests. They should always return the request content and allow
> the underlying WSGI server or hosting web server to discard the
> request content."
>

Interesting. I hadn't taken the wsgi implementation into account. This
seems to preclude app-servers from implementing HEAD with any shortcuts
(e.g., cache the headers). Anyway, we'll leave this alone then...


>
> * My original implementation of the "head" method on the View base
> class [1] *did* check whether the view already has a "get" method.
> This was later changed in [2] to the current semantics which, I agree,
> looks a bit wrong. Perhaps we should revert to the original behaviour?
>
> [1] https://github.com/django/django/commit/c7fe43
> [2] https://github.com/django/django/commit/760d0f


Yeah, that implementation seems preferable. What was the reason for backing
it out?

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

New feature: HTTP OPTIONS and improved HEAD for View

[Steven Cummings]

I'd like to propose some simple improvements to
django.views.generic.base.View:

1. To complement the automatic HTTP 405 response, provide a default
implementation of the HTTP OPTIONS method. This default response would have
an HTTP 204 NO CONTENT status and include an Allow response header just
like that of the 405 response (django.http.HttpResponseNotAllowed).
Developers could easily override this method to provide more info about
endpoint or server capabilities.

2. Improve the default HTTP HEAD handler by ensuring it has a blank body,
as this is the expectation of a response to a HEAD request. Also, there
should be consideration for the GET handler not existing, which should
result in a 405. The code appears to assume it exists, which would result
in an AttributeError instead.

If these are generally agreed upon I'll supply the patch.
--
Steven

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Update on Ticket #16891: Delete/update should return number of rows modified

[Steven Cummings]

Okay, that's good stuff. So make the QuerySet APIs consistent
(delete/update should return the same sort of info) but leave Model alone
entirely.
--
Steven


On Sat, Nov 26, 2011 at 3:58 PM, Florian Apolloner wrote:

> I think it would be great if delete would return the deleted rows (but
> only of the base table obviously), -1 on model.save() returning a rowcount.
>
>
> Cheers,
> Florian
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/django-developers/-/ziNdv-aRePQJ.
>
> To post to this group, send email to django-developers@googlegroups.com.
> To unsubscribe from this group, send email to
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Update on Ticket #16891: Delete/update should return number of rows modified

[Steven Cummings]

Right, let me rephrase that as just Model.save(), Model.delete(), &
QuerySet.delete()
--
Steven


On Sat, Nov 26, 2011 at 3:37 PM, Florian Apolloner <f.apollo...@gmail.com>wrote:

> Hi,
>
>
> On Saturday, November 26, 2011 9:42:38 PM UTC+1, Steven Cummings wrote:
>>
>> So, what are core-dev thoughts on going on ahead and returning
>> row-modified counts from Model.save() and QuerySet.update/delete?
>>
>
> QuerySet.update already returns the modified row count.
>
> Cheers,
> Florian
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/django-developers/-/cubG4FAps0oJ.
>
> To post to this group, send email to django-developers@googlegroups.com.
> To unsubscribe from this group, send email to
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Update on Ticket #16891: Delete/update should return number of rows modified

[Steven Cummings]

I'm assuming there's some more work to do here, but I was hoping to get
opinions from core devs on what's been discussed so far. Some of the
feedback above was that because save() can only return a rows-modified
count of 1 right now, that it's silly to even change. So the tentative
decision is simply to have no changes to the Model and QuerySet APIs at all
right now (a generally safe decision, all other things aside).

So, what are core-dev thoughts on going on ahead and returning row-modified
counts from Model.save() and QuerySet.update/delete? If not, are there any
comments on the code changes to date (which are fairly minimal), or is this
something that everybody would be generally okay with me wrapping up very
soon? Is this generally something that there is low interest in because my
ultimate use-case (avoiding optimistic concurrency problems) is a non-issue
for some majority of Django developers?

Thanks!

--
Steven


On Mon, Oct 17, 2011 at 10:02 PM, Steven Cummings <estebis...@gmail.com>wrote:

> Well, per discussion so far I've rolled the changes back [1] to simply
> ensuring that the low-level query objects return usable counts. There was
> discussion of whether delete should return counts for immediate objects
> only or related as well, but I further decided to simply impose no API
> changes on base Model for now.
>
> [1]
> https://github.com/estebistec/django/commit/f178b72af132dd1ba588b89fe6915f5e9ba841d0
> --
> Steven
>
>
>
> On Sun, Oct 16, 2011 at 4:48 PM, Steven Cummings <estebis...@gmail.com>wrote:
>
>> Any opinions from core devs before I go back and make adjustments, or are
>> some waiting to see the patch before weighing in?
>>
>> --
>> Steven
>>
>>
>>
>> On Thu, Oct 13, 2011 at 11:48 PM, Steven Cummings 
>> <estebis...@gmail.com>wrote:
>>
>>> On Thu, Oct 13, 2011 at 1:06 AM, Anssi Kääriäinen <
>>> anssi.kaariai...@thl.fi> wrote:
>>>>
>>>> Now I have the feeling that I have gone through this exact same
>>>> discussion before, and have had the exact same misunderstanding, too,
>>>> before. So, sorry for that...
>>>>
>>>
>>> It's cool. Better to make sure we're all clear here on the different
>>> opinions and options.
>>>
>>>
>>>>  > I think a reasonable option to discuss might be leaving the save()
>>>> API as it
>>>> > is and rolling this enhancement back to the internal code (i.e.,
>>>> > UpdateQuery, DeleteQuery) returning counts to support the prospective
>>>> > enhancements I've alluded to, and/or overrides of save(). Until there
>>>> are
>>>> > any changes to save(), I agree it is not going to be useful info.
>>>> However
>>>> > for delete it seems immediately usable (and then we're left with the
>>>> debate
>>>> > of counting immediate-only or including related objects).
>>>>
>>>> I would go with immediate only, with the ability to get the counts for
>>>> cascaded deletes per object type as a kwarg option. The kwarg option
>>>> could be left for later implementation. One reason for immediate only
>>>> is that at least PostgreSQL and MySQL does it that way for ON DELETE
>>>> CASCADE foreign keys. So, if you are getting the value from the cursor
>>>> and using ON DELETE CASCADE instead of doing the cascade in Django,
>>>> you will not get the cascade counts anyways. And even if you do the
>>>> cascade in Django, then it would be consistent with what a SQL
>>>> database would report.
>>>
>>>
>>> Well, by those conventions and all of the feedback I've gotten so far,
>>> counting only immediate objects in a delete seems like the best way.
>>>
>>>  Are there other opinions on this issue or the return value of save? Do
>>> any core devs have insight into any potential changes in the ORM that would
>>> be affected by this decision?
>>>
>>> Thanks.
>>>
>>
>>
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Update on Ticket #16891: Delete/update should return number of rows modified

[Steven Cummings]

Well, per discussion so far I've rolled the changes back [1] to simply
ensuring that the low-level query objects return usable counts. There was
discussion of whether delete should return counts for immediate objects only
or related as well, but I further decided to simply impose no API changes on
base Model for now.

[1]
https://github.com/estebistec/django/commit/f178b72af132dd1ba588b89fe6915f5e9ba841d0
--
Steven


On Sun, Oct 16, 2011 at 4:48 PM, Steven Cummings <estebis...@gmail.com>wrote:

> Any opinions from core devs before I go back and make adjustments, or are
> some waiting to see the patch before weighing in?
>
> --
> Steven
>
>
>
> On Thu, Oct 13, 2011 at 11:48 PM, Steven Cummings <estebis...@gmail.com>wrote:
>
>> On Thu, Oct 13, 2011 at 1:06 AM, Anssi Kääriäinen <
>> anssi.kaariai...@thl.fi> wrote:
>>>
>>> Now I have the feeling that I have gone through this exact same
>>> discussion before, and have had the exact same misunderstanding, too,
>>> before. So, sorry for that...
>>>
>>
>> It's cool. Better to make sure we're all clear here on the different
>> opinions and options.
>>
>>
>>>  > I think a reasonable option to discuss might be leaving the save() API
>>> as it
>>> > is and rolling this enhancement back to the internal code (i.e.,
>>> > UpdateQuery, DeleteQuery) returning counts to support the prospective
>>> > enhancements I've alluded to, and/or overrides of save(). Until there
>>> are
>>> > any changes to save(), I agree it is not going to be useful info.
>>> However
>>> > for delete it seems immediately usable (and then we're left with the
>>> debate
>>> > of counting immediate-only or including related objects).
>>>
>>> I would go with immediate only, with the ability to get the counts for
>>> cascaded deletes per object type as a kwarg option. The kwarg option
>>> could be left for later implementation. One reason for immediate only
>>> is that at least PostgreSQL and MySQL does it that way for ON DELETE
>>> CASCADE foreign keys. So, if you are getting the value from the cursor
>>> and using ON DELETE CASCADE instead of doing the cascade in Django,
>>> you will not get the cascade counts anyways. And even if you do the
>>> cascade in Django, then it would be consistent with what a SQL
>>> database would report.
>>
>>
>> Well, by those conventions and all of the feedback I've gotten so far,
>> counting only immediate objects in a delete seems like the best way.
>>
>>  Are there other opinions on this issue or the return value of save? Do
>> any core devs have insight into any potential changes in the ORM that would
>> be affected by this decision?
>>
>> Thanks.
>>
>
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Update on Ticket #16891: Delete/update should return number of rows modified

[Steven Cummings]

Any opinions from core devs before I go back and make adjustments, or are
some waiting to see the patch before weighing in?

--
Steven


On Thu, Oct 13, 2011 at 11:48 PM, Steven Cummings <estebis...@gmail.com>wrote:

> On Thu, Oct 13, 2011 at 1:06 AM, Anssi Kääriäinen <anssi.kaariai...@thl.fi
> > wrote:
>>
>> Now I have the feeling that I have gone through this exact same
>> discussion before, and have had the exact same misunderstanding, too,
>> before. So, sorry for that...
>>
>
> It's cool. Better to make sure we're all clear here on the different
> opinions and options.
>
>
>>  > I think a reasonable option to discuss might be leaving the save() API
>> as it
>> > is and rolling this enhancement back to the internal code (i.e.,
>> > UpdateQuery, DeleteQuery) returning counts to support the prospective
>> > enhancements I've alluded to, and/or overrides of save(). Until there
>> are
>> > any changes to save(), I agree it is not going to be useful info.
>> However
>> > for delete it seems immediately usable (and then we're left with the
>> debate
>> > of counting immediate-only or including related objects).
>>
>> I would go with immediate only, with the ability to get the counts for
>> cascaded deletes per object type as a kwarg option. The kwarg option
>> could be left for later implementation. One reason for immediate only
>> is that at least PostgreSQL and MySQL does it that way for ON DELETE
>> CASCADE foreign keys. So, if you are getting the value from the cursor
>> and using ON DELETE CASCADE instead of doing the cascade in Django,
>> you will not get the cascade counts anyways. And even if you do the
>> cascade in Django, then it would be consistent with what a SQL
>> database would report.
>
>
> Well, by those conventions and all of the feedback I've gotten so far,
> counting only immediate objects in a delete seems like the best way.
>
> Are there other opinions on this issue or the return value of save? Do any
> core devs have insight into any potential changes in the ORM that would be
> affected by this decision?
>
> Thanks.
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Update on Ticket #16891: Delete/update should return number of rows modified

[Steven Cummings]

On Thu, Oct 13, 2011 at 1:06 AM, Anssi Kääriäinen
wrote:
>
> Now I have the feeling that I have gone through this exact same
> discussion before, and have had the exact same misunderstanding, too,
> before. So, sorry for that...
>

It's cool. Better to make sure we're all clear here on the different
opinions and options.


> > I think a reasonable option to discuss might be leaving the save() API as
> it
> > is and rolling this enhancement back to the internal code (i.e.,
> > UpdateQuery, DeleteQuery) returning counts to support the prospective
> > enhancements I've alluded to, and/or overrides of save(). Until there are
> > any changes to save(), I agree it is not going to be useful info. However
> > for delete it seems immediately usable (and then we're left with the
> debate
> > of counting immediate-only or including related objects).
>
> I would go with immediate only, with the ability to get the counts for
> cascaded deletes per object type as a kwarg option. The kwarg option
> could be left for later implementation. One reason for immediate only
> is that at least PostgreSQL and MySQL does it that way for ON DELETE
> CASCADE foreign keys. So, if you are getting the value from the cursor
> and using ON DELETE CASCADE instead of doing the cascade in Django,
> you will not get the cascade counts anyways. And even if you do the
> cascade in Django, then it would be consistent with what a SQL
> database would report.


Well, by those conventions and all of the feedback I've gotten so far,
counting only immediate objects in a delete seems like the best way.

Are there other opinions on this issue or the return value of save? Do any
core devs have insight into any potential changes in the ORM that would be
affected by this decision?

Thanks.

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Update on Ticket #16891: Delete/update should return number of rows modified

[Steven Cummings]

On Wed, Oct 12, 2011 at 11:51 PM, Anssi Kääriäinen
wrote:
>
>
> My point is that we should never just return 0 if there is a
> concurrent modification or some other reason for not being able to
> persist the object state. Why? When you request obj.save() you are
> requesting the object state to be persisted. If that can't be done,
> it's an exception. delete() does not have the same problem: if you do
> a delete, and the object is already deleted, then there is no problem.
> The DB state is still synchronized with the delete.
>

I'm not sure anybody said 0 would be returned for an *inability* to save
data. I would agree that would be an exception situation, but it hasn't been
discussed here yet.


>
> The biggest problem of just returning 0 from the .save() on concurrent
> modification is that if the developer forgets to do if obj.save(): ...
>
>
So the situation I expect this to occur in is the context of very deliberate
coding by the programmer, e.g.:

count = obj.save(where={'version': 1})

If the programmer forgets to check the count then they probably didn't
understand the feature being used to being with. I don't think this is going
to result in the accidents you're describing, because until there is a
specific feature context, save will return 1 (or whatever else we decide).
I've no intention of introducing implicit features or pitfalls.

>
> Also, if you just return 0, you will lose the information WHY the
> object couldn't be saved, you just know it wasn't saved for some
> reason.


> Another way to think about this: when there is a unique constraint
> violation, we raise an exception (the one from the DB backend). To be
> consistent with returning 0 when the object can't be saved due to some
> other reason, shouldn't we just catch that exception and return 0? Bad
> idea, right?
>

I generally agree with this, but as I stated above I've yet to discuss a
situation involving the inability to save. In the example so far the reason
for not saving is the condition given by the programmer. I won't be
swallowing any exceptions and returning zero, and if a coder chooses to
avoid saving on a version for optimistic concurrency, that is a decision,
not an inability.


> > So, is there a good sense of "sure" or "probable" enhancements to models
> > that could help us with the return API of Model.save, and whether
> > QuerySet.delete should return counts that include related objects? If
> > "coming enhancements" or directions for Models can concretely inform,
> that
> > would be great.
>
> I don't think there is. For that reason I would just postpone the
> model.save() return value decision. I feel pretty strongly that
> model.save() should never return 0 in the meaning of "didn't save your
> changes" (note that this is different from "there were no changes").
> It is just too easy to miss that return value and have half-done
> updates because of that.
>

I think a reasonable option to discuss might be leaving the save() API as it
is and rolling this enhancement back to the internal code (i.e.,
UpdateQuery, DeleteQuery) returning counts to support the prospective
enhancements I've alluded to, and/or overrides of save(). Until there are
any changes to save(), I agree it is not going to be useful info. However
for delete it seems immediately usable (and then we're left with the debate
of counting immediate-only or including related objects).

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Update on Ticket #16891: Delete/update should return number of rows modified

[Steven Cummings]

--
Steven


On Wed, Oct 12, 2011 at 8:07 AM, Anssi Kääriäinen
wrote:

>
> Model.save() should return models.UPDATED or models.INSERTED. That is
> more helpful than the 1/0 return value. For example you might want to
> do different things if the model was inserted or updated in
> application code. Or if loading data from a file, you might want to
> know how many objects already existed in the DB and how many were
> inserted. Of course if you could cheaply get models.UNCHANGED, that
> would make the feature perfect...
>

If you're ensuring a set of data from some other source, what's the use of
knowing what was new? Are there other concrete cases where this might be
useful?


>
> I agree that model.save() should never return 0/None/models.NOACTION,
> that should be an exception instead of a return value. After .save()
> the model should exists in the database. If the object for some reason
> isn't persisted, it is worth an exception instead of easy to miss
> return value.
>

Possibly, but an exception cannot be added until the programmer is passing a
precondition and can reasonable expect the condition of nothing being saved.
That's definitely not on this ticket.


>
>  - Anssi Kääriäinen
>
> --
> 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
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Update on Ticket #16891: Delete/update should return number of rows modified

[Steven Cummings]

--
Steven


On Wed, Oct 12, 2011 at 5:37 AM, Johannes Dollinger <emulb...@googlemail.com
> wrote:

>
> Am 12.10.2011 um 03:35 schrieb Steven Cummings:
>


> Ticket #12086 is a duplicate. I don't think QuerySet.delete() should return
> the number of all collected objects, but just the number of objects deleted
> from QuerySet.model.
>

Is the idea that you should just assume related objects are deleted, and so
the count should just reflect the current set of top-level objects in the
QuerySet? I ask because I'm actually not sure about this one myself.

And Model.save()/Model.delete() should really only return 1, as *one object*
> has been saved/deleted, even if more than one database row was touched.
> Thus, both methods don't need the return value.
>

See my response to Florian. The result may not always be one after further
enhancements.


> Thanks for working on this!
> __
> Johannes
>
> --
> 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
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Update on Ticket #16891: Delete/update should return number of rows modified

[Steven Cummings]

It is for now, but the ticket I'm building up to [1] is that it may actually
be 0 if a precondition is not met. This is how your code can know if the
current request really got to do a save or delete if two or more are trying
to update the object at the same time.

[1] https://code.djangoproject.com/ticket/16549
--
Steven


On Wed, Oct 12, 2011 at 4:28 AM, Florian Apolloner wrote:

> Uhm question -- Does the count for Model.save() make any sense? Isn't that
> always one?
>
> Cheers,
> Florian
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers" group.
> To view this discussion on the web visit
> https://groups.google.com/d/msg/django-developers/-/rpkcIupLI9gJ.
> To post to this group, send email to django-developers@googlegroups.com.
> To unsubscribe from this group, send email to
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Update on Ticket #16891: Delete/update should return number of rows modified

[Steven Cummings]

Finally got around to these simple changes:

*
https://github.com/estebistec/django/commit/b48a87afc324f5546b6654fa7638e406b397c0d6
*
https://github.com/estebistec/django/commit/28ace32980b370fd17ae35019bfe8d055c673684

If the core devs approve of these changes I can get to work on creating the
proper patch for SVN and add doc changes.

Ticket: https://code.djangoproject.com/ticket/16891#comment:3
--
Steven

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Improvements to better support implementing optimistic concurrency control

[Steven Cummings]

On Thu, Aug 11, 2011 at 12:06 PM, Steven Cummings <estebis...@gmail.com>wrote:

> On Thu, Aug 11, 2011 at 11:14 AM, Simon Riggs <si...@2ndquadrant.com>wrote:
>
>> IDEA 2
>> (1) SELECT data & version
>> (2) UPDATE data & test version & COMMIT immediately
>>
>
> This is pretty much where I started when I initiated the ticket and thread
> and still am at. The other discussion here is what happens when the test
> fails (i.e., transaction control).
>

Also,  I was hesitant that my "version" example really covered everybody's
needs, which is why I was also leaning towards generalizing to gaining some
access to rows modified.

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Improvements to better support implementing optimistic concurrency control

[Steven Cummings]

On Thu, Aug 11, 2011 at 11:14 AM, Simon Riggs  wrote:

> IDEA 2
> (1) SELECT data & version
> (2) UPDATE data & test version & COMMIT immediately
>

This is pretty much where I started when I initiated the ticket and thread
and still am at. The other discussion here is what happens when the test
fails (i.e., transaction control).

As I show above, the proposed patch doesn't solve the problem, I am
> sorry to say.
>

It appears I need to go ahead and get started on my patch which would pretty
much be idea 2.


>
> Yes, not updating atomically is a problem. Doing the updates
> atomically could also create a deadlock risk. It's a hard problem.
>

Agreed.

The very best way to do this from the database perspective is to use
> the SQL Standard RETURNING clause, which allows you to return the data
> after the update
>
> IDEA5
> UPDATE foo
> SET val = x
> WHERE pk = y
> RETURNING data
>
> but I can see that causing a few discussion points, so I didn't
> mention it previously.
>

This I'm not familiar with, so I'll need to look at that.

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Improvements to better support implementing optimistic concurrency control

[Steven Cummings]

I don't think we're talking about new or specific fields as part of the base
implementation here. Just enhanced behavior around updates to:

1) Provide more information about the actual rows modified
2) Check preconditions with the actual DB stored values; and
3) Avoid firing post-update/delete signals if nothing was changed

>From there you could implement fields as you see fit for your app, e.g.,
version=IntegerField() that you use in a precondition.
--
Steven


On Mon, Aug 8, 2011 at 3:57 PM, akaariai <akaar...@cc.hut.fi> wrote:

> On Aug 8, 6:30 pm, Steven Cummings <estebis...@gmail.com> wrote:
> > For backward compatibility, there may be a Model sub-class that would
> leave
> > Model alone altogether (this was suggested on the ticket). This seems
> fair
> > since many seem to be getting by without better optimistic concurrency
> > control from Django's ORM today.
>
> Would the subclass-based method automatically append a field into the
> model, or would there be need to also create the field used for
> version control? How does the subclass know which field to use?
>
> Yet another option is models.OptimisticLockField(). If there is one
> present in the model, and a save will result in update, the save
> method will check for conflicts and set the version to version + 1 if
> there are no conflicts. There is some precedent for a somewhat similar
> field, the AutoField(). AutoField also changes how save behaves.
>
> I wonder what to do if the save does not result in an update and the
> version is set to something else than 1. This could happen if another
> user deleted the model and you are now saving it. This would result in
> a reinsert. This should also be an error?
>
>  - Anssi
>
> --
> 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
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: Improvements to better support implementing optimistic concurrency control

[Steven Cummings]

For backward compatibility, there may be a Model sub-class that would leave
Model alone altogether (this was suggested on the ticket). This seems fair
since many seem to be getting by without better optimistic concurrency
control from Django's ORM today.
--
Steven


On Mon, Aug 8, 2011 at 9:40 AM, akaariai <akaar...@cc.hut.fi> wrote:

> On Aug 8, 4:54 pm, Steven Cummings <estebis...@gmail.com> wrote:
> > Interesting feature I hadn't noticed in memcached. That does seem like it
> > would do the trick where memcached is being used. I think the ability to
> > control it in Django would generally still be desirable though, as that
> is
> > where the data ultimately lives and I'd be hesitant to assume to control
> the
> > DB's concurrency from memcached. Ideally it should be the other way
> around.
>
> I assume the memcached implementation would be a version value stored
> in memcached. Can you really trust that memcached keeps the version
> value and doesn't discard it at will when it has been unused long
> enough?
>
> There are a couple of other things in model saving which could be
> better handled. If composite primary keys are included in Django, one
> would need the ability to update the primary key. If you have a model
> with (first_name, last_name) primary key, and you change the
> first_name and save, current implementation (and definition) of model
> save() would insert a new instance into the DB instead of doing an
> update. Another thing that could be handled better is update of just
> the changed fields.
>
> I wonder how to implement these things with backwards compatibility.
> Maybe a method update(condition=None, only_fields=None) which returns
> True if something was actually updated (or raises an exception if
> nothing was updated). The method would use the old pk and the
> condition (if given) in the where clause. If only_fields=None, then it
> would only update the changed fields... Seems uqly, but I can't think
> of anything better.
>
>  - Anssi
>
> --
> 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
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Re: [RFC] Improvements to better support implementing optimistic concurrency control

[Steven Cummings]

Interesting feature I hadn't noticed in memcached. That does seem like it
would do the trick where memcached is being used. I think the ability to
control it in Django would generally still be desirable though, as that is
where the data ultimately lives and I'd be hesitant to assume to control the
DB's concurrency from memcached. Ideally it should be the other way around.
--
Steven


On Sun, Aug 7, 2011 at 8:00 PM, Peter Portante
<peter.a.porta...@gmail.com>wrote:

> Essentially, you want a compare-and--swap instruction for a database?
>
> Have you considered using memcached atomicity (add and cas) to handle this
> kind of thing? It might get pretty elaborate, but with just a cursory
> thought seems doable.
>
> -peter
>
>
> On Sat, Aug 6, 2011 at 9:59 PM, Steven Cummings <estebis...@gmail.com>wrote:
>
>> Bad community member that I am, I jumped the gun and logged a ticket
>> [1] on this one... anyway:
>>
>> Websites general avoid overly aggressive locking of data to deal with
>> concurrency issues. When two users allowed to edit some data are
>> simultaneously doing so, they're both allowed to do so and the
>> last .save() wins (race condition). This is largely acceptable on the
>> web as when two people are allowed to edit data they shouldn't be
>> prevented to doing so. Further sites often have audit history (whether
>> or not exposed to those users owning/managing the data) which tracks
>> exactly what went on.
>>
>> However, some sites may wish to detect this situation and present the
>> 2nd user with a page or the previous form stating that what they
>> edited is now obsolete. Examples of situations where we don't want
>> edits to be silently stomped are health and financial data. Bugzilla
>> mid-air collisions are an example of an implementation in the wild.
>>
>> So, we could try to detect this by always re-querying the object just
>> before update (or delete), but with sufficient traffic there is still
>> the chance that two requests *think* they know the current state of it
>> in storage. This is where knowing rows modified, and/or having a
>> precondition check would help. I've outlined the details on the
>> ticket, but generally:
>>
>> * To ensure the object I'm saving is not stale, I might like to do
>> Model.save_if(version=version), where I have an incremented version-
>> field and save_if would compare the stored value (using something like
>> an F('version')) against the given value. If the version was
>> different, it should raise something like PreconditionFailed.
>> * For delete()s, it might be nice to get rows-modified to know that
>> the current request really did perform the delete. This is important
>> in cases like using an OAuth auth code, where allowing multiple uses
>> (deletes) of it would be a security problem.
>>
>> For both these cases, it would also be useful to either get rows-
>> modified (or precondition-met) in the post-save/delete signal, or to
>> somehow avoid those signals firing when no data was updated.
>>
>> I'm fully willing to hack on this and provide patches and/or use an
>> experimental branch, just wanted to get some thoughts on it.
>>
>> [1] https://code.djangoproject.com/ticket/16549
>>
>> --
>> 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
>> django-developers+unsubscr...@googlegroups.com.
>> For more options, visit this group at
>> http://groups.google.com/group/django-developers?hl=en.
>>
>>
>  --
> 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
> django-developers+unsubscr...@googlegroups.com.
> For more options, visit this group at
> http://groups.google.com/group/django-developers?hl=en.
>

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

[RFC] Improvements to better support implementing optimistic concurrency control

[Steven Cummings]

Bad community member that I am, I jumped the gun and logged a ticket
[1] on this one... anyway:

Websites general avoid overly aggressive locking of data to deal with
concurrency issues. When two users allowed to edit some data are
simultaneously doing so, they're both allowed to do so and the
last .save() wins (race condition). This is largely acceptable on the
web as when two people are allowed to edit data they shouldn't be
prevented to doing so. Further sites often have audit history (whether
or not exposed to those users owning/managing the data) which tracks
exactly what went on.

However, some sites may wish to detect this situation and present the
2nd user with a page or the previous form stating that what they
edited is now obsolete. Examples of situations where we don't want
edits to be silently stomped are health and financial data. Bugzilla
mid-air collisions are an example of an implementation in the wild.

So, we could try to detect this by always re-querying the object just
before update (or delete), but with sufficient traffic there is still
the chance that two requests *think* they know the current state of it
in storage. This is where knowing rows modified, and/or having a
precondition check would help. I've outlined the details on the
ticket, but generally:

* To ensure the object I'm saving is not stale, I might like to do
Model.save_if(version=version), where I have an incremented version-
field and save_if would compare the stored value (using something like
an F('version')) against the given value. If the version was
different, it should raise something like PreconditionFailed.
* For delete()s, it might be nice to get rows-modified to know that
the current request really did perform the delete. This is important
in cases like using an OAuth auth code, where allowing multiple uses
(deletes) of it would be a security problem.

For both these cases, it would also be useful to either get rows-
modified (or precondition-met) in the post-save/delete signal, or to
somehow avoid those signals firing when no data was updated.

I'm fully willing to hack on this and provide patches and/or use an
experimental branch, just wanted to get some thoughts on it.

[1] https://code.djangoproject.com/ticket/16549

-- 
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 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.