Re: release? FileSession.py

[Jim Gallacher <jg...@sympatico.ca>]

Nicolas Lehuen wrote:
> Jim,
> 
> Are you OK with this version of FileSession.py ? 

No, I moved beyond that version a while ago.

> I'm not sure I
> totally followed your thoughts since at a point the thread on
> FileSession became a bit blurry to me.

Hardly a suprise. It was getting kind of convoluted to me as well.

> If it's not OK, please send me
> a corrected version.

Attached is my latest and hopefully greatest. :)  This version uses the 
grace_period idea to avoid race and/or deadlock issues and correctly 
register the filesession_cleanup method to run at the end of a request.

In the original code there was a comment (from dharana?) about a random 
EOFError in the do_load method. I have not been able to reproduce this 
error but I left the comment in anyway.

Also, my comments for the filesession_cleanup function are rather 
verbose. Feel free to trim them as you see fit.

Regards,
Jim



> Regards,
> Nicolas
> 
> On 4/27/05, Jim Gallacher <jg...@sympatico.ca> wrote:
> 
>>Gregory (Grisha) Trubetskoy wrote:
>>
>>>What do you folks think about gearing up for a release? Meaning not
>>>adding new features and focusing on the little details (like docs,
>>>tests, bugs).
>>
>>+1
>>
>>
>>>The only major thing that I think needs to be finalized is FileSession -
>>>I'd like it to see all the locking issues resolved and either the code
>>>rolled in to Session.py or Session.py split into separate files by
>>>class, for consistency. I also think that if FileSession works well, it
>>>should be made the default session mechanism.
>>
>>I think the last patch I submitted (2005-04-21) takes care of the
>>locking issues, but I don't think it's been committed yet. There is
>>still one small change I'd like to make in the do_load method, but other
>>than that I think the code is in good shape.
>>
>>I like the idea of separate files for each class since it would make it
>>easy to add additional backends if there is such a desire in the future.
>>
>>
>>>Anything else major that I'm missing? I'd like to start a little
>>>discussion on what would be a reasonable timeline and start making baby
>>>steps towards achieving it.
>>>
>>
>>I'll be away until Sunday, but should be able to put together some
>>documentation for FileSession and do some additional testing early next
>>week.
>>
>>Regards,
>>Jim
>>

Re: release? FileSession.py

[Jim Gallacher <jg...@sympatico.ca>]

Nicolas Lehuen wrote:
> On 4/28/05, Nicolas Lehuen <ni...@gmail.com> wrote:
> 
>>On 4/27/05, dharana <dh...@dharana.net> wrote:
>>
>>>Jim Gallacher wrote:
>>>
>>>>Nicolas Lehuen wrote:
>>>>
>>>>[...]
>>>>
>>>>Attached is my latest and hopefully greatest. :)  This version uses the
>>>>grace_period idea to avoid race and/or deadlock issues and correctly
>>>>register the filesession_cleanup method to run at the end of a request.
>>
>>Thanks, I'll integrate this ASAP tomorrow morning (it's too late for now :).
> 
> 
> OK, this is done.
> 
> 
>>>>In the original code there was a comment (from dharana?) about a random
>>>>EOFError in the do_load method. I have not been able to reproduce this
>>>>error but I left the comment in anyway.
>>>
>>>No, it's not mine. I have yet to see it happen.
>>
>>It was mine. The bug is totally reproducible, on my machine at least.
>>I'll investigate this a little more tomorrow.
> 
> 
> Found it. It turns out that we need to open the session file in binary
> mode for reading and writing.
> 
> I've never really understood why, but on Win32 files can be opened in
> text mode, in which some control characters mean an EOF. If you try to
> write binary data in a text-mode file, and your binary data contains
> an EOF character, then you're out of luck. You have to open the file
> in binary mode, by adding an 'b' to the mode.
> 
> What lead to to find this was that when using the text-friendly
> pickling protocol 0, there was no problem. The bug only appeared when
> using the binary pickling protocol 2.
> 

Ah, yes. One of those cross-platform wrinkles. Glad the fix was simple.

Jim

Re: release? FileSession.py

[Nicolas Lehuen <ni...@gmail.com>]

Wooops, I've integrated FileSession.py into Session.py because I
thought the subject was settled. Anyway, it's not so difficult to
split it again. Only this time, I'll make sure that everyone agrees...

Regards
Nicolas

On 4/30/05, Jim Gallacher <jg...@sympatico.ca> wrote:
> Gregory (Grisha) Trubetskoy wrote:
> >
> > On Thu, 28 Apr 2005, Nicolas Lehuen wrote:
> >
> >> OK so now I think we have a pretty good implementation of FileSession :)
> >
> >
> > Excellent - now who's in favor of splitting Session into multiple files
> > versus rolling FileSession inti Session.py?
> 
> +1 for splitting.
> 
> > I'm thinking that if the Session.py is split, then same should be done
> > with Cookie.py for consistency and that's too much changes when we are
> > in a pre-release stage.
> 
> Agreed. Since this is mainly a bug fix release, except for FileSession,
>   I wouldn't shift things around too much.
> 
> > So I'm in favor of adding FileSession into Session.py for _this_ release
> > with the plan to consider splitting Session.py and Cookie.py later.
> 
> I'm not sure I like the idea of merging FileSession into Session and
> then splitting it back out in a future release. This just does not seem
> all that graceful. If the plan is to split Session, then start with
> FileSession as it will not impact any current code. When the ultimate
> future split occurs at least some application code will not be broken.
> 
> Regards,
> Jim
>

Re: release? FileSession.py

[Jim Gallacher <jg...@sympatico.ca>]

Gregory (Grisha) Trubetskoy wrote:
> 
> On Thu, 28 Apr 2005, Nicolas Lehuen wrote:
> 
>> OK so now I think we have a pretty good implementation of FileSession :)
> 
> 
> Excellent - now who's in favor of splitting Session into multiple files 
> versus rolling FileSession inti Session.py?

+1 for splitting.

> I'm thinking that if the Session.py is split, then same should be done 
> with Cookie.py for consistency and that's too much changes when we are 
> in a pre-release stage.

Agreed. Since this is mainly a bug fix release, except for FileSession, 
  I wouldn't shift things around too much.

> So I'm in favor of adding FileSession into Session.py for _this_ release 
> with the plan to consider splitting Session.py and Cookie.py later.

I'm not sure I like the idea of merging FileSession into Session and 
then splitting it back out in a future release. This just does not seem 
all that graceful. If the plan is to split Session, then start with 
FileSession as it will not impact any current code. When the ultimate 
future split occurs at least some application code will not be broken.

Regards,
Jim

Re: release? FileSession.py

["Gregory (Grisha) Trubetskoy" <gr...@ispol.com>]

On Thu, 28 Apr 2005, Nicolas Lehuen wrote:

> OK so now I think we have a pretty good implementation of FileSession :)

Excellent - now who's in favor of splitting Session into multiple files 
versus rolling FileSession inti Session.py?

I'm thinking that if the Session.py is split, then same should be done 
with Cookie.py for consistency and that's too much changes when we are in 
a pre-release stage.

So I'm in favor of adding FileSession into Session.py for _this_ release 
with the plan to consider splitting Session.py and Cookie.py later.

Grisha

Re: release? FileSession.py

[Nicolas Lehuen <ni...@gmail.com>]

On 4/28/05, Nicolas Lehuen <ni...@gmail.com> wrote:
> On 4/27/05, dharana <dh...@dharana.net> wrote:
> > Jim Gallacher wrote:
> > > Nicolas Lehuen wrote:
> > >
> > > [...]
> > >
> > > Attached is my latest and hopefully greatest. :)  This version uses the
> > > grace_period idea to avoid race and/or deadlock issues and correctly
> > > register the filesession_cleanup method to run at the end of a request.
> 
> Thanks, I'll integrate this ASAP tomorrow morning (it's too late for now :).

OK, this is done.

> > > In the original code there was a comment (from dharana?) about a random
> > > EOFError in the do_load method. I have not been able to reproduce this
> > > error but I left the comment in anyway.
> >
> > No, it's not mine. I have yet to see it happen.
> 
> It was mine. The bug is totally reproducible, on my machine at least.
> I'll investigate this a little more tomorrow.

Found it. It turns out that we need to open the session file in binary
mode for reading and writing.

I've never really understood why, but on Win32 files can be opened in
text mode, in which some control characters mean an EOF. If you try to
write binary data in a text-mode file, and your binary data contains
an EOF character, then you're out of luck. You have to open the file
in binary mode, by adding an 'b' to the mode.

What lead to to find this was that when using the text-friendly
pickling protocol 0, there was no problem. The bug only appeared when
using the binary pickling protocol 2.

OK so now I think we have a pretty good implementation of FileSession :)

Regards,
Nicolas

> > >
> > > Also, my comments for the filesession_cleanup function are rather
> > > verbose. Feel free to trim them as you see fit.
> > >
> > > Regards,
> > > Jim
> >
> > I have tested this version's performance which was the driving force
> > behind the initial suggestion. Performance-wise, this FileSession
> > version is as fast as the first rough one without locking, grace times,
> > etc. The difference is less than 1 req/s in the 165 req/s' range.
> >
> > I've switched my dev app to it and i will be using it daily from now on.
> >
> > --
> > dharana
> 
> Great ! Please give us your feedback on the behaviour of the new FileSession.
> 
> Regards,
> 
> Nicolas
>

Re: release? FileSession.py

[Nicolas Lehuen <ni...@gmail.com>]

On 4/27/05, dharana <dh...@dharana.net> wrote:
> Jim Gallacher wrote:
> > Nicolas Lehuen wrote:
> >
> > [...]
> >
> > Attached is my latest and hopefully greatest. :)  This version uses the
> > grace_period idea to avoid race and/or deadlock issues and correctly
> > register the filesession_cleanup method to run at the end of a request.

Thanks, I'll integrate this ASAP tomorrow morning (it's too late for now :).

> > In the original code there was a comment (from dharana?) about a random
> > EOFError in the do_load method. I have not been able to reproduce this
> > error but I left the comment in anyway.
>
> No, it's not mine. I have yet to see it happen.

It was mine. The bug is totally reproducible, on my machine at least.
I'll investigate this a little more tomorrow.

> >
> > Also, my comments for the filesession_cleanup function are rather
> > verbose. Feel free to trim them as you see fit.
> >
> > Regards,
> > Jim
> 
> I have tested this version's performance which was the driving force
> behind the initial suggestion. Performance-wise, this FileSession
> version is as fast as the first rough one without locking, grace times,
> etc. The difference is less than 1 req/s in the 165 req/s' range.
> 
> I've switched my dev app to it and i will be using it daily from now on.
> 
> --
> dharana

Great ! Please give us your feedback on the behaviour of the new FileSession.

Regards,

Nicolas

Re: release? FileSession.py

[dharana <dh...@dharana.net>]

Jim Gallacher wrote:
> Nicolas Lehuen wrote:
> 
> [...]
> 
> Attached is my latest and hopefully greatest. :)  This version uses the 
> grace_period idea to avoid race and/or deadlock issues and correctly 
> register the filesession_cleanup method to run at the end of a request.
> 
> In the original code there was a comment (from dharana?) about a random 
> EOFError in the do_load method. I have not been able to reproduce this 
> error but I left the comment in anyway.

No, it's not mine. I have yet to see it happen.

> 
> Also, my comments for the filesession_cleanup function are rather 
> verbose. Feel free to trim them as you see fit.
> 
> Regards,
> Jim

I have tested this version's performance which was the driving force 
behind the initial suggestion. Performance-wise, this FileSession 
version is as fast as the first rough one without locking, grace times, 
etc. The difference is less than 1 req/s in the 165 req/s' range.

I've switched my dev app to it and i will be using it daily from now on.

-- 
dharana