At 08:58 PM 6/19/00 +0800, mike wrote:
http://www.zope.org/Members/RainDog/ZSession/ZSession-0.0.2.tar.gz/view
Comments?
Now that I've had a chance to really look at this (while tracking down one of the bugs you found), I do have a few comments. First, nice job... It's a good adaptation use of ZPatterns. I do have some suggestions for fine tuning, though. * Rather than putting property manipulation functions right into the base Session class, I would simply let users derive ZClasses of their own from Session. They could then actually give the Session a UI of its own, for example. Think of someone creating a Session subclass called "Shopping Cart", with methods for viewing, checking out, adding/deleting items, etc. Or, if they have many subsystems which want to share the same Session objects, they could do this by having each subsystem use a different propertysheet of the same Session object. IMHO, the basicSheet stuff you've done, while convenient, encourages developers to take the quick-and-dirty route of just using the Session as a place to store variables - and that throws away a lot of the usefulness of ZPatterns. For one thing, you've guaranteed that Sessions have to be stored as persistent objects in the ZODB, or at least that the basicSheet has to support arbitrary properties. * Make dead session removal something that doesn't happen automatically, or at least have an option to do so. Your default deletion algorithm would be very expensive when executed against a non-ZODB database, and would be more efficiently done a few times a day in large batches by a cron job. * Delegate determining which sessions are dead to the Rack, rather than doing it in the Specialist. This allows an SQL implementation to make use of indexes. Specifically, make removeDead call: self._getSource().findIdleSessions(self.session_ttl) And then provide a default implementation of findIdleSessions() in the Specialist. (Btw, please note that your current implementation absolutely requires that at least *some* session data be stored persistently in the ZODB, which if you're using a FileStorage, is a bad place for it to be.) Now, the whole thing works the same way when first installed, but if someone implements the session ID and lastAccessed attributes in an SQL database, they can substitute a different implementation for findIdleSessions() by putting it in the Rack.