Request for review - connection loss in transactions

2014-03-24 Thread Aymeric Augustin
Hello, Like its predecessors `commit_on_success` and `xact`, `transaction.atomic` doesn't guarantee atomicity if the connection gets closed in the middle of the transaction. Django automatically reopens it and proceeds happily, losing the first half of the transaction and saving only the second

Re: Request for review / Multiple time zone support for datetime representation

2013-09-08 Thread Aymeric Augustin
Hello, As noted by Luke in his review of the time zone support branch, there's a large overlap between django.utils.tzinfo and django.utils.timezone. See the excerpt of our discussion below. I prepared a pull request for this long overdue refactoring. https://github.com/django/django/pull/1601

Re: Request for review: database-level autocommit

2013-03-09 Thread Aymeric Augustin
On 9 mars 2013, at 00:51, Florian Apolloner wrote: > If you don't care about the timeframe I'd like to smoke test it on > Postgresql, Mysql, Oracle on Tuesday (again -- due to an exam I won't have > time for earlier tests [although I am pretty sure postgres and mysql are in a > good shape aft

Re: Request for review: database-level autocommit

2013-03-08 Thread Florian Apolloner
Hi Aymeric, On Friday, March 8, 2013 11:32:52 PM UTC+1, Aymeric Augustin wrote: > > Carl was kind enough to review the branch in detail. (Thank you!) > > I'll take his feedback into account, clean up the history, and push a new > version tomorrow. I hope to merge it during the week-end. > If you

Re: Request for review: database-level autocommit

2013-03-08 Thread Aymeric Augustin
On 7 mars 2013, at 17:37, Aymeric Augustin wrote: > Barring unexpected problems and last minute vetoes, I plan to merge > this branch when another core dev reviews it thoroughly and accepts it, Carl was kind enough to review the branch in detail. (Thank you!) I'll take his feedback into accou

Re: Request for review: database-level autocommit

2013-03-07 Thread Aymeric Augustin
On 7 mars 2013, at 17:37, Aymeric Augustin wrote: > My work on transaction management is now ready for review: > https://github.com/django/django/pull/873 It also fixes at least ten tickets. Here's a short explanation for those I found. #2227: `atomic` supports nesting. #6623: `commit_manual

Request for review: database-level autocommit

2013-03-07 Thread Aymeric Augustin
My work on transaction management is now ready for review: https://github.com/django/django/pull/873 To sum up, the main improvements over the current implementation are: 1) A better API for transactions - Simple: just one function covers most of the use cases - Correct: it actua

Re: Request for review / Multiple time zone support for datetime representation

2013-02-13 Thread Aymeric Augustin
Le 13 févr. 2013 à 20:54, Ian Kelly a écrit : > I only meant that the final SQL should use '0:00' instead of > 'UTC' to guard against databases that for whatever reason do not have > a UTC time zone definition, such as the one that I found. Otherwise > the query would always fail with an ORA-0188

Re: Request for review / Multiple time zone support for datetime representation

2013-02-13 Thread Ian Kelly
On Tue, Feb 12, 2013 at 3:35 PM, Aymeric Augustin wrote: >> Against the possible issue of non-recognition of UTC, I suggest using >> '0:00' instead of 'UTC'. > > Thanks for looking into this problem! > > I tried '0:00' instead of 'UTC', and '+03:00' instead of > 'Africa/Nairobi' with the same resu

Re: Request for review / Multiple time zone support for datetime representation

2013-02-13 Thread Aymeric Augustin
I bit the bullet and implemented passing the time zone name: - via a parameter on PostgreSQL, SQLite and MySQL, - through string interpolation on Oracle, with a regexp to validate the value. As a pre-requisite, the ORM needs to handle parameters in the SELECT clause: https://github.com/aa

Re: Request for review / Multiple time zone support for datetime representation

2013-02-12 Thread Aymeric Augustin
Le 12 févr. 2013 à 23:12, Ian Kelly a écrit : > On Tue, Feb 12, 2013 at 3:06 PM, Ian Kelly wrote: >> >> I encounter the same bug in Oracle 10g XE. I tried it also on an 11.2 >> database, and it seemed to work, but I ran into a different issue: >> neither of the time zones 'Africa/Nairobi' nor

Re: Request for review / Multiple time zone support for datetime representation

2013-02-12 Thread Ian Kelly
On Tue, Feb 12, 2013 at 3:06 PM, Ian Kelly wrote: > On Tue, Feb 12, 2013 at 2:25 PM, Aymeric Augustin > wrote: > c.execute("""SELECT "DT" FROM "TIMEZONES_EVENT" WHERE EXTRACT(MONTH FROM > CAST((FROM_TZ("DT", 'UTC') AT TIME ZONE (:arg)) AS DATE)) = 1""", > ['Africa/Nairobi']).fetchal

Re: Request for review / Multiple time zone support for datetime representation

2013-02-12 Thread Ian Kelly
On Tue, Feb 12, 2013 at 2:25 PM, Aymeric Augustin wrote: c.execute("""SELECT "DT" FROM "TIMEZONES_EVENT" WHERE EXTRACT(MONTH FROM CAST((FROM_TZ("DT", 'UTC') AT TIME ZONE (:arg)) AS DATE)) = 1""", ['Africa/Nairobi']).fetchall() > Traceback (most recent call last): > File "", line

Re: Request for review / Multiple time zone support for datetime representation

2013-02-12 Thread Aymeric Augustin
I spent the better part of the day struggling with Oracle and unsurprisingly nothing works :( Given this model: class Event(models.Model): dt = models.DateTimeField() I'm trying to implement this query with time zone support enabled: Event.objects.filter(dt__month=1) That's a

Re: Request for review / Multiple time zone support for datetime representation

2013-02-11 Thread Aymeric Augustin
Le 1 nov. 2011 à 15:58, Aymeric Augustin a écrit : > > There is only one outstanding issue that I know of. `QuerySet.dates()` > operates in the database timezone, ie. UTC, while an end user would expect it > to take into account its current time zone. This affects `date_hierarchy` in > the ad

Request for review: removal of deprecated features for 1.6

2012-12-24 Thread Aymeric Augustin
Hello, Since we forked the 1.5.x branch from master when we released 1.5 alpha, we can work on 1.6 while we finalize 1.5. I've created a branch that removes all deprecated code. Here's a pull request for inline comments: https://github.com/django/django/pull/605 Specifically, I'd love a sanity

Re: Request for review for a small fix in the csrf view

2012-02-21 Thread Paul McMillan
> In short, the first patch add a bullet point in the CSRF error page > which states that this > error can be triggered by disabled cookies. I committed this change. > The second patch fixes the middleware itself to make the page show the > correct error message if the > error is caused by disabl

Request for review for a small fix in the csrf view

2012-02-20 Thread h3
I've submitted a ticket[1] with two patches as a follow-up to this discussion: http://groups.google.com/group/django-developers/browse_thread/thread/ca34924871e3c00b/b29cd0e17c010f54?lnk=gst&q=csrf+cookie+haineault#b29cd0e17c010f54 In short, the first patch add a bullet point in the CSRF error pa

Re: Request for review / Multiple time zone support for datetime representation

2011-11-19 Thread Hanne Moa
On 18 November 2011 16:57, Anssi Kääriäinen wrote: > My point is that it would be very useful to know you are actually > using aware datetimes internally. Especially when migrating an old > project to use this feature, it is easy to miss some places that do > not use aware datetimes. And you might

Re: Request for review / Multiple time zone support for datetime representation

2011-11-18 Thread Aymeric Augustin
2011/11/18 Anssi Kääriäinen > My point is that it would be very useful to know you are actually > using aware datetimes internally. Especially when migrating an old > project to use this feature, it is easy to miss some places that do > not use aware datetimes. And you might find it out when you

Re: Request for review / Multiple time zone support for datetime representation

2011-11-18 Thread Anssi Kääriäinen
On Nov 18, 5:31 pm, Aymeric Augustin wrote: > Hi Anssi, > > 2011/11/18 Anssi Kääriäinen > > > So, at least according to this simple test there is nothing to > > worry about performance-wise. > > Thanks for the performance tests! They were still on my TODO list. The tests could be better... It mi

Re: Request for review / Multiple time zone support for datetime representation

2011-11-18 Thread Aymeric Augustin
Hi Anssi, 2011/11/18 Anssi Kääriäinen > So, at least according to this simple test there is nothing to > worry about performance-wise. > Thanks for the performance tests! They were still on my TODO list. > This part of the documentation raises one question: > """ > Unfortunately, during DST

Re: Request for review / Multiple time zone support for datetime representation

2011-11-18 Thread Anssi Kääriäinen
On Nov 1, 4:58 pm, Aymeric Augustin wrote: > I'm glad to announce that the time zone support branch is ready for review. I did a simple performance test on PostgreSQL (fetch 1000 objs with datetime field) and could not see any real performance regressions. I did the same for template rendering, a

Re: Request for review / Multiple time zone support for datetime representation

2011-11-11 Thread Luke Plant
Just in case you were waiting for a reply, I've read your response and everything seems good to me. Luke On 10/11/11 07:46, Aymeric Augustin wrote: > Hi Luke, > > Thanks for the review! I've updated the branch with your suggestions. > > Here are a few comments -- when I have something to say o

Re: Request for review / Multiple time zone support for datetime representation

2011-11-10 Thread Luke Plant
On 10/11/11 07:46, Aymeric Augustin wrote: >> I haven't actually reviewed any of the test code - I ran out of energy, >> and test code is so dull, sorry! >> >> I did run coverage for the tests and found: >> >> - without pytz installed, the new LocalTimezone class > > You apparently ran out of ene

Re: Request for review / Multiple time zone support for datetime representation

2011-11-09 Thread Aymeric Augustin
Hi Luke, Thanks for the review! I've updated the branch with your suggestions. Here are a few comments -- when I have something to say other than "fixed, thanks". 2011/11/8 Luke Plant : > == django/contrib/admin/util.py == > >>    elif isinstance(field, models.DateField): >>        return format

Re: Request for review / Multiple time zone support for datetime representation

2011-11-08 Thread Luke Plant
Hi Aymeric, Finally got around to this! My review follows. I didn't follow the original discussions very closely, so forgive any questions that have been answered already. On the other hand, the code and comments probably ought to answer the kind of questions I have, without needing to search mail

Request for review / Multiple time zone support for datetime representation

2011-11-01 Thread Aymeric Augustin
Hello, I'm glad to announce that the time zone support branch is ready for review. Review the changes: https://bitbucket.org/aaugustin/django/compare/..django/django Clone the branch: hg clone https://bitbucket.org/aaugustin/django Play with the demo app: hg clone https://bitbucket.org/aaugustin

Re: #12991 Adding support for unittest2: request for review

2010-09-26 Thread Russell Keith-Magee
On Monday, September 27, 2010, Chuck Harmston wrote: > I just ran it on Python 2.4, 2.5, 2.6, and 2.7 with each of SQLite, MySQL, > and PostgreSQL on Debian with no problems. Awesome! Thanks for that, Chuck. Yours, Russ Magee %-) -- You received this message because you are subscribed to the

Re: #12991 Adding support for unittest2: request for review

2010-09-26 Thread Chuck Harmston
I just ran it on Python 2.4, 2.5, 2.6, and 2.7 with each of SQLite, MySQL, and PostgreSQL on Debian with no problems. Thanks for the work you put into this, Russell! On Sun, Sep 26, 2010 at 6:24 PM, Karen Tracey wrote: > I've run it on Python 2.4 & 2.5 (Ubuntu, sqlite DB) with no problems. > >

Re: #12991 Adding support for unittest2: request for review

2010-09-26 Thread Russell Keith-Magee
On Monday, September 27, 2010, Patrick Altman wrote: > > On Sep 26, 2010, at 7:44 PM, Russell Keith-Magee wrote: > >> On Mon, Sep 27, 2010 at 6:24 AM, Karen Tracey wrote: >>> I've run it on Python 2.4 & 2.5 (Ubuntu, sqlite DB) with no problems. >>> >>> I do have some feedback on the @skipIfDB add

Re: #12991 Adding support for unittest2: request for review

2010-09-26 Thread Patrick Altman
On Sep 26, 2010, at 7:44 PM, Russell Keith-Magee wrote: > On Mon, Sep 27, 2010 at 6:24 AM, Karen Tracey wrote: >> I've run it on Python 2.4 & 2.5 (Ubuntu, sqlite DB) with no problems. >> >> I do have some feedback on the @skipIfDB addition: I'd really like if this >> could be used to distinguis

Re: #12991 Adding support for unittest2: request for review

2010-09-26 Thread Alex Gaynor
On Sun, Sep 26, 2010 at 8:44 PM, Russell Keith-Magee wrote: > On Mon, Sep 27, 2010 at 6:24 AM, Karen Tracey wrote: >> I've run it on Python 2.4 & 2.5 (Ubuntu, sqlite DB) with no problems. >> >> I do have some feedback on the @skipIfDB addition: I'd really like if this >> could be used to distingu

Re: #12991 Adding support for unittest2: request for review

2010-09-26 Thread Russell Keith-Magee
On Mon, Sep 27, 2010 at 6:24 AM, Karen Tracey wrote: > I've run it on Python 2.4 & 2.5 (Ubuntu, sqlite DB) with no problems. > > I do have some feedback on the @skipIfDB addition: I'd really like if this > could be used to distinguish between the different MySQL storage backends. > From a very bri

Re: #12991 Adding support for unittest2: request for review

2010-09-26 Thread Karen Tracey
I've run it on Python 2.4 & 2.5 (Ubuntu, sqlite DB) with no problems. I do have some feedback on the @skipIfDB addition: I'd really like if this could be used to distinguish between the different MySQL storage backends. >From a very brief look it seems like currently there are a bunch of tests ski

#12991 Adding support for unittest2: request for review

2010-09-26 Thread Russell Keith-Magee
Hi all, I've just uploaded a patch for #12991, adding support for unittest2 to Django. This is a very big patch (500kb, 12k lines). For this reason, I've uploaded the patch compressed. Trac won't allow attachments greater than 200k, and given past history, large diffs break Trac anyway. However,

Request for review

2008-06-11 Thread Collin Grady
Not sure exactly how to phrase this, but James told me to bring these tickets up here to get some feedback on them - such as what exactly needs done to bring them to 'ready for checkin' as myself and others would like to get them in :) I did the basics of making sure the patches were updated,