Re: Switch to database-level autocommit

[Shai Berger]

On Friday 01 March 2013, Aymeric Augustin wrote:
> Hello,
> 
> I'd like to improve transactions handling in Django. The first step is [to
> replace -- assumed, SB] the current emulation of autocommit with
> database-level autocommit.
> 

There is an issue you seem to be ignoring: An "ORM Write" is, in many cases, 
more than a single query against the backend. The most obvious example is 
models with inheritance -- trying to do these with database-level autocommit 
means that the code writes the two parts in separate transactions. I'm very -1 
on this.

There are two alternatives I see -- one is to make sure that an ORM Write is 
still a transaction (not database-level autocommit); the other is to 
explicitly commit-unless-managed (or something equivalent) after every read 
and raw-sql, as well as write.

BTW, how do you treat select-for-update, which currently makes sense in "faked 
autocommit" mode, but will stop making sense with the suggested change?

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Switch to database-level autocommit

[Shai Berger]

Thinking again, a few more points come to mind:

On Friday 01 March 2013, Aymeric Augustin wrote:
>
> Such transactions are useless and don't come for free. Relying on them to
> enforce integrity is extremely fragile — what if an external library
> starts writing to a log table in the middle of one of these implicit
> transactions? 

Then it's the external library that is at fault; it should be using a separate 
alias, with its own transactions.

> The term "footgun" comes to mind.
> 
True; but has a wider implication than you seem to give it credit for. In 
particular,

> 
> ** Backwards-compatibility **
> 
> Roughly, I'd classify Django users in four groups:
> 1 - "Transactions, how do they work?"
> 2 - "Django autocommits, doesn't it?"
> 3 - "I'm using TransactionMiddleware!"
> 4 - "I'm managing my transactions."
> 
> Groups 1 and 2 won't see the difference.

This will turn existing projects from correct (though perhaps fragile) to 
buggy, exactly in groups 1&2, where the users are less likely to understand 
the issues. The apps are correct today -- they rely on the documented 
transaction behavior, even if the authors are not fully aware of it. And they 
will turn buggy in a way that is not likely to be detected by tests, because 
it will have effects mostly under concurrent access or system crashes -- 
circumstances you don't usually have in tests.

Thus,
> 
> I don't see much point in providing an option to turn autocommit off,
> because starting a transaction is a much more explicit way to achieve the
> same effect. We usually don't provide "backwards compatibility with bugs".
> 
-1. Make it easier (and cross-backend) to set db-level-autocommit on. Put the 
setting for it in the default template for new projects. Don't change existing 
code from "fragile" to "subtly broken".

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Switch to database-level autocommit

[Aymeric Augustin]

On 2 mars 2013, at 15:50, Shai Berger  wrote:

> There is an issue you seem to be ignoring: An "ORM Write" is, in many cases, 
> more than a single query against the backend. The most obvious example is 
> models with inheritance -- trying to do these with database-level autocommit 
> means that the code writes the two parts in separate transactions. I'm very 
> -1 
> on this.

Yes, that's very important. Anssi raised this problem on IRC too.

(I also heard that Django is currently broken in this regard; I didn't check.)

> There are two alternatives I see -- one is to make sure that an ORM Write is 
> still a transaction (not database-level autocommit); the other is to 
> explicitly commit-unless-managed (or something equivalent) after every read 
> and raw-sql, as well as write.

The plan is to ensure that ORM operations that may result in multiple queries
are executed within a transaction.

It's possible to start with a dumb implementation, ie. wrap all ORM writes in a
transaction, and eventually get smarter, ie. determine if more than one query
will be emitted.

Even the dumb implementation will have better transactional properties than
the status quo.

In practice, that means adding support for savepoints to all backends, ripping
off https://github.com/xof/xact — with permission from Christophe, which I
haven't asked for yet — and wrapping ORM writes in this decorator. However,
autocommit is a pre-requisite for adding savepoints to SQLite, because of the
stdlib's braindead implementation of PEP 249.

Hence, autocommit is part 1 of the plan, and improved transaction
management is part 2.

> BTW, how do you treat select-for-update, which currently makes sense in 
> "faked 
> autocommit" mode, but will stop making sense with the suggested change?


I'm leaning towards not doing any magic and documenting this as a backwards
incompatibility. Relying on Django's current pseudo-auto-commit is very fragile:

qs = MyModel.objects.filter(…)
qs.select_for_update(…)
# what if someone introduces an unrelated ORM write here?
qs.update(…)

I hope developers already wrap their select_for_update calls and associated
writes in transaction.commit_on_success, and it's a good thing to make it
mandatory.

-- 
Aymeric.



-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Switch to database-level autocommit

[Aymeric Augustin]

On 2 mars 2013, at 16:18, Shai Berger  wrote:

> -1. Make it easier (and cross-backend) to set db-level-autocommit on. Put the 
> setting for it in the default template for new projects. Don't change 
> existing 
> code from "fragile" to "subtly broken".

This isn't simply about changing some default. It's about re-implementing all 
the
transaction machinery.

With the scope I described, I estimate that I can complete the project in 120 
to 200
hours. If I have to keep bug-parity with the current behavior, I estimate that 
I'm not
capable of delivering it.

In other words, you're requesting that I fork Django rather than contribute 
this to the
main tree.

-- 
Aymeric.



-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Switch to database-level autocommit

[Shai Berger]

On Saturday 02 March 2013, Aymeric Augustin wrote:
> On 2 mars 2013, at 16:18, Shai Berger  wrote:
> > -1. Make it easier (and cross-backend) to set db-level-autocommit on. Put
> > the setting for it in the default template for new projects. Don't
> > change existing code from "fragile" to "subtly broken".
> 
> This isn't simply about changing some default. It's about re-implementing
> all the transaction machinery.
> 
> With the scope I described, I estimate that I can complete the project in
> 120 to 200 hours. If I have to keep bug-parity with the current behavior,
> I estimate that I'm not capable of delivering it.
> 
> In other words, you're requesting that I fork Django rather than contribute
> this to the main tree.

I may have phrased my objection too strongly -- all in all, I laud your 
continuing effort to improve Django and its database layer, including this 
change. However, I must stand my ground.

The Django documentation on transactions, at the moment says this on Django's 
default behavior[0]:

> Django’s default behavior is to run with an open transaction which it
> commits automatically when any built-in, data-altering model function is
> called. For example, if you call model.save() or model.delete(), the
> change will be committed immediately.

Now, you want to call this behavior (the part where there's one open 
transaction, rather than one-per-db-operation) a bug. I find that a little odd, 
but I can live with it.

What I have a harder time living with is my other point, which has not been 
refuted: The proposed change turns code that is currently correct -- liable to 
break if changed, but still correct as it stands -- into code with "phantom 
bugs" that show up only in production, under specific timings of concurrent 
requests (because reads and writes which used to be in the same transaction 
are now on separate transactions). It is my opinion that such changes may only 
be made when changing major versions (i.e. Django 2.0), if at all. 

If anyone can offer a way to detect the situation and warn users of the change, 
that would make things perfectly fine. Changing the default transaction 
behavior along the lines you suggested would be a great improvement. I have a 
strong suspicion, though, that if the new behavior is implemented as default 
(and with no recourse to the old behavior, to boot), then that simply cannot 
be done.

I think that as suggested, the change would break the project's committments 
to its users in a way that is much more important than the performance gains, 
avoided idle-in-transaction errors, and effort considerations, all put 
together. That is my objection.

Thanks for your attention,

Shai.

[0] https://docs.djangoproject.com/en/dev/topics/db/transactions/

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Transforming django-admin.py to a shell script

[Alon Nisser]

much simpler. simply making it run with simple `manage somecommand` instead 
of `python manage.py somecommand`. using setuptools or something similiar

On Saturday, March 2, 2013 12:17:25 AM UTC+2, Łukasz Rekucki wrote:
>
> On 1 March 2013 22:38, Alon Nisser >wrote:
>
>> at least from my windows exprience with Django (yes, I know this isn't a 
>> common use case, but still) the current django-admin.py and manage.py do 
>> need python preface to run right (while inside a virtualenv)
>
>
> I'm not 100% sure if the Windows Python Launcher[1] works with virtualenv, 
> but it most likely works with venv support in Python 3.3. It should also 
> allow you to run Python scripts without the .py suffix
>
> Anyways, what kind of "shell" you had in mind? Batch ? Powershell ? Bash ?.
>
> [1]: http://blog.python.org/2011/07/python-launcher-for-windows_11.html
>  

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Switch to database-level autocommit

[Aymeric Augustin]

On 2 mars 2013, at 21:46, Shai Berger  wrote:

> The Django documentation on transactions, at the moment says this on Django's 
> default behavior[0]:
> 
>> Django’s default behavior is to run with an open transaction which it
>> commits automatically when any built-in, data-altering model function is
>> called. For example, if you call model.save() or model.delete(), the
>> change will be committed immediately.

Yes, my proposal is a backwards-incompatible change with regard to this
sentence.

However, I believe that only a small fraction of the users of Django fully
grok the implications of this sentence, and a fraction of this fraction relies
on it to ensure some level of integrity.

> What I have a harder time living with is my other point, which has not been 
> refuted: The proposed change turns code that is currently correct -- liable 
> to 
> break if changed, but still correct as it stands -- into code with "phantom 
> bugs" that show up only in production, under specific timings of concurrent 
> requests (because reads and writes which used to be in the same transaction 
> are now on separate transactions).

I'm aware of this scenario and I'm willing to document it in the release
notes.

> It is my opinion that such changes may only 
> be made when changing major versions (i.e. Django 2.0), if at all.

The current consensus — at least within the core team — is that there will never
be a "break everything" Django 2.0.

I believe that the level of backwards-incompatibility described above is within
acceptable bounds for Django 1.6.

I also believe that it beats the alternative — namely, live with the current
behavior forever.

> If anyone can offer a way to detect the situation and warn users of the 
> change, 
> that would make things perfectly fine. Changing the default transaction 
> behavior along the lines you suggested would be a great improvement. I have a 
> strong suspicion, though, that if the new behavior is implemented as default 
> (and with no recourse to the old behavior, to boot), then that simply cannot 
> be done.
> 
> I think that as suggested, the change would break the project's committments 
> to its users in a way that is much more important than the performance gains, 
> avoided idle-in-transaction errors, and effort considerations, all put 
> together. That is my objection.


Yes, I agree with this way to frame the discussion.

We reach different conclusions because I don't consider backwards
compatibility an absolute that trumps every other consideration. It's a
continuum. Backwards compatibility must find a balance with progress.  The
same goes for security: while extremely important, it still has to find a
balance with usability — otherwise you'd never dare open port 80.

-- 
Aymeric.



-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Switch to database-level autocommit

[Shai Berger]

Hi again,

On Sunday 03 March 2013, Aymeric Augustin wrote:
> On 2 mars 2013, at 21:46, Shai Berger  wrote:
> > The Django documentation on transactions, at the moment says this on
> > Django's
> > 
> > default behavior[0]:
> >> Django’s default behavior is to run with an open transaction which it
> >> commits automatically when any built-in, data-altering model function is
> >> called. For example, if you call model.save() or model.delete(), the
> >> change will be committed immediately.
> 
> Yes, my proposal is a backwards-incompatible change with regard to this
> sentence.
> 
> However, I believe that only a small fraction of the users of Django fully
> grok the implications of this sentence, and a fraction of this fraction
> relies on it to ensure some level of integrity.
> 

I agree with this belief about users *deliberately* relying on the 
implications for integrity. I have no idea how many people simply wrote code 
that works, and will now break. I also have no idea how many people maintain a 
system started long ago by a more knowledgeable developer.

> > What I have a harder time living with is my other point, which has not
> > been refuted: The proposed change turns code that is currently correct
> > -- liable to break if changed, but still correct as it stands -- into
> > code with "phantom bugs" that show up only in production, under specific
> > timings of concurrent requests (because reads and writes which used to
> > be in the same transaction are now on separate transactions).
> [...]
> 
> I believe that the level of backwards-incompatibility described above is
> within acceptable bounds for Django 1.6.
> 

I believe this is the core of our disagreement here.

> I also believe that it beats the alternative — namely, live with the
> current behavior forever.
> 

I sincerely hope that is not the only alternative; that there's a way to 
implement the new behavior side-by-side with the old one. There usually is.

> > If anyone can offer a way to detect the situation and warn users of the
> > change, that would make things perfectly fine. Changing the default
> > transaction behavior along the lines you suggested would be a great
> > improvement. I have a strong suspicion, though, that if the new behavior
> > is implemented as default (and with no recourse to the old behavior, to
> > boot), then that simply cannot be done.
> > 
> > I think that as suggested, the change would break the project's
> > committments to its users in a way that is much more important than the
> > performance gains, avoided idle-in-transaction errors, and effort
> > considerations, all put together. That is my objection.
> 
> Yes, I agree with this way to frame the discussion.
> 
> We reach different conclusions because I don't consider backwards
> compatibility an absolute that trumps every other consideration. It's a
> continuum. Backwards compatibility must find a balance with progress.

I don't consider backwards-compatibility an absolute, and the first of my two 
paras which you quoted above shows just that. I do think there are proper ways 
to introduce backwards-incompatible changes, and what you suggested isn't one 
of them.

In particular, per https://docs.djangoproject.com/en/dev/misc/api-stability/, 
we should have a deprecation process with warnings, unless fixing a security 
bug. Even with security issues, it was deemed preferrable to take an expedited 
deprecation process when possible (e.g. with the markdown module) rather than 
an immediate breaking change. Throwing this process to the wind over an 
improvement -- as nice as it is -- seems unwise to me.

An immeiately-breaking change, where the breakage is hard to detect and debug, 
and with high potential for data loss... I don't think any warning in the 
release notes can be enough. All I can say is, -1.

Shai.

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Switch to database-level autocommit

[Jacob Kaplan-Moss]

On Sat, Mar 2, 2013 at 3:27 PM, Shai Berger  wrote:
>> I believe that the level of backwards-incompatibility described above is
>> within acceptable bounds for Django 1.6.
>
> I believe this is the core of our disagreement here.

I'm with Aymeric: the current behavior is bad enough, and this is a
big enough improvement, and the backwards-incompatibility is minor
enough. I think the data-loss possibilities Shai suggests are pretty
overblown. Shai: do you have any real-world code that'd have data loss
with this bug? Looking through the tons of code I've got available
can't reveal a single place where this chance would have any negative
effect. OTOH, there're lots of places where it'd have a positive
effect.

Jacob

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Switch to database-level autocommit

[Florian Apolloner]

Hi Shai,

On Sunday, March 3, 2013 12:27:47 AM UTC+1, Shai Berger wrote:
>
> > I also believe that it beats the alternative — namely, live with the 
>
> > current behavior forever.
>   
>
> I sincerely hope that is not the only alternative; that there's a way to 
> implement the new behavior side-by-side with the old one. There usually is.
>

Can you describe how an alternative in this case would look like? I 
personally can't think of any sensible one.
 

> I don't consider backwards-compatibility an absolute, and the first of my 
> two paras which you quoted above shows just that. I do think there are 
> proper ways to introduce backwards-incompatible changes, and what you 
> suggested isn't one of them.
>

If you think there are proper ways to introduce this specific change, 
please show us. Aside from that, all of our bugfixes and new features have 
a possibility to break code. Bugfixes that break old code usually occur as 
a side effect of not seeing the issue the fix causes or being fully aware 
of the issue and considering it minor enough (that is no data-loss and only 
happening to a small fraction of users). New Features shouldn't break code, 
but they usually also touch old code, so they can break stuff in the same 
way as bugfixes.

In the case of this change I think that users with low traffic sites won't 
even notice the changes (no concurrency) and users with high traffic sites 
shouldn't see it cause they should be using autocommit and the relevant 
decorators anyways…


An immeiately-breaking change, where the breakage is hard to detect and 
> debug, and with high potential for data loss... I don't think any warning 
> in the release notes can be enough. All I can say is, -1.
>

I am still +1.

Cheers,
Florian

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.

Re: Transforming django-admin.py to a shell script

[Joe Tennies]

So, you are asking for the startproject (which again, you only run once, so
I wouldn't think it would be TOO bad) to create a manage.bat that had the
following line in it?

python manage.py %*

I'm not saying that this is a bad idea. It does make it fairly easy for
Windows people. In the case of Linux and Mac, I think the old way "just
works" in 90+% of the cases.

That stated, could we get the following lines added to virtualenv's
activate.bat:

assoc .py=Python.File
ftype Python.File=(magic path to python.exe or pythonw.exe)

>From what I'm reading*, that would solve this problem not just for Django
but for everyone. I'll create a patch for virtualenv once I get a chance to
test this theory.

*(
http://stackoverflow.com/questions/8012956/python-django-virtualenv-windows)




On Sat, Mar 2, 2013 at 2:57 PM, Alon Nisser  wrote:

> much simpler. simply making it run with simple `manage somecommand`
> instead of `python manage.py somecommand`. using setuptools or something
> similiar
>
> On Saturday, March 2, 2013 12:17:25 AM UTC+2, Łukasz Rekucki wrote:
>
>> On 1 March 2013 22:38, Alon Nisser  wrote:
>>
>>> at least from my windows exprience with Django (yes, I know this isn't a
>>> common use case, but still) the current django-admin.py and manage.py do
>>> need python preface to run right (while inside a virtualenv)
>>
>>
>> I'm not 100% sure if the Windows Python Launcher[1] works with
>> virtualenv, but it most likely works with venv support in Python 3.3. It
>> should also allow you to run Python scripts without the .py suffix
>>
>> Anyways, what kind of "shell" you had in mind? Batch ? Powershell ? Bash
>> ?.
>>
>> [1]: http://blog.python.org/**2011/07/python-launcher-for-**
>> windows_11.html
>>
>  --
> You received this message because you are subscribed to the Google Groups
> "Django developers" 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?hl=en
> .
> For more options, visit https://groups.google.com/groups/opt_out.
>
>
>



-- 
Joe Tennies
tenn...@gmail.com

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" 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?hl=en.
For more options, visit https://groups.google.com/groups/opt_out.