Re: Ticket #21751 review requested

2014-02-02 Thread Michael Manfre
The pull request has been rebased. Thanks, Michael Manfre On Sun, Feb 2, 2014 at 9:40 AM, Aymeric Augustin < aymeric.augus...@polytechnique.org> wrote: > Hi Michael, > > On 21 janv. 2014, at 19:05, Michael Manfre wrote: > > The overall feedback I've gotten from my pull

Re: Ticket #21751 review requested

2014-02-02 Thread Aymeric Augustin
Hi Michael, On 21 janv. 2014, at 19:05, Michael Manfre wrote: > The overall feedback I've gotten from my pull request seems positive. I'll > rebase it against master and submit it for merging. Let me know when you do this. I'll push the merge button before the patch gets

Re: Ticket #21751 review requested

2014-01-26 Thread Shai Berger
Just to make things clear, my reservations about the use of 'with' syntax with cursors do not amount to an objection to this PR. I have not reviewed it again since the rebase, but I did like most of what I've seen before. The with- blocks can be cleaned up later, if I or anyone else find it

Re: Ticket #21751 review requested

2014-01-21 Thread Michael Manfre
I agree with Aymeric in that cursors are very much like files. Wrapping a single line of code in a with (or a try-finally) is a standard pattern and allows Django to follow PEP 249 without extra modifications. The overall feedback I've gotten from my pull request seems positive. I'll rebase it

Re: Ticket #21751 review requested

2014-01-19 Thread Shai Berger
On Sunday 19 January 2014 10:44:32 Michael Manfre wrote: > On Sun, Jan 19, 2014 at 5:23 AM, Shai Berger wrote: > > Still, spreading with-blocks all over the code for this looks very ugly to > > me. > > I'd rather add an execute_single() or execute_and_close() method to the > >

Re: Ticket #21751 review requested

2014-01-19 Thread Michael Manfre
On Sun, Jan 19, 2014 at 5:23 AM, Shai Berger wrote: > > Still, spreading with-blocks all over the code for this looks very ugly to > me. > I'd rather add an execute_single() or execute_and_close() method to the > cursors instead. Perhaps even only as private API, as that usage

Re: Ticket #21751 review requested

2014-01-19 Thread Shai Berger
On Sunday 19 January 2014 11:52:40 Łukasz Rekucki wrote: > On 19 January 2014 09:12, Shai Berger wrote: > > On Friday 17 January 2014 01:19:29 Michael Manfre wrote: > >> In an effort to make Django a bit more explicit with regards to closing > >> cursors when they are no longer

Re: Ticket #21751 review requested

2014-01-19 Thread Łukasz Rekucki
On 19 January 2014 09:12, Shai Berger wrote: > On Friday 17 January 2014 01:19:29 Michael Manfre wrote: >> In an effort to make Django a bit more explicit with regards to closing >> cursors when they are no longer needed, I have created ticket #21751 [1] >> with a pull

Re: Ticket #21751 review requested

2014-01-19 Thread Aymeric Augustin
On 19 janv. 2014, at 09:12, Shai Berger wrote: > I think this is suboptimal, API-wise. Python destroys temporaries soon > enough. > Is there a reason why we cannot arrange for a __del__ method to close the > cursor? Circular references anywhere? Hi Shai, You probably

Re: Ticket #21751 review requested

2014-01-19 Thread Shai Berger
On Friday 17 January 2014 01:19:29 Michael Manfre wrote: > In an effort to make Django a bit more explicit with regards to closing > cursors when they are no longer needed, I have created ticket #21751 [1] > with a pull request[2]. > > Most of the changes are straight forward and change the usage

Ticket #21751 review requested

2014-01-16 Thread Michael Manfre
In an effort to make Django a bit more explicit with regards to closing cursors when they are no longer needed, I have created ticket #21751 [1] with a pull request[2]. Most of the changes are straight forward and change the usage of a cursor so that it uses a context manager or closes the cursor