Titus Brown wrote:

>-> I ran test_session.py with MySQL (see below), Durus, shelve, and directory
>-> storage, and didn't get any errors.  test_session.py is a Quixote
>-> application with a number-incrementing link and login/logout links.
>-> Because the store selector is hardcoded in create_publisher(), I had to
>-> change it in the source between each run.  Observations:
>->
>-> - For MySQL, I modified SQLSessionStore.__init__ to take an already-open
>-> connection from the caller.  This shrinks the method to two lines and
>-> eliminates the module's dependency on psycopg.
>
>OK, sounds good.
>
>-> - This code is so much more straightforward and flexible than the default
>-> Quixote code I'd consider it a significant improvement.  It makes it easy
>-> to define additional session stores in the future.
>
>I'm glad to hear it!
>
>As you may have noticed, PersistentSessionManager is the innovation.
>The particular SessionStores are, umm, crudely written ;).
>
>-> - Session.is_dirty() is apparently not used anywhere.  This means I don't
>-> have to subclass Session anymore to use it with persistent stores?
>-> Hooray.
>
>Yes: were this needed, it could be left to an interaction between the
>SessionStore and Session class.
>
>-> - Since these classes are so small, why not put them all in one module?
>-> They can import their pecular stuff in their methods, to avoid the module
>-> depending on all libraries.
>
>I _much_ prefer short, concise files that contain only the functionality
>used; it makes it easier for new people to grok code.
>
>I also think that imports Should happen at the top of modules, wherever
>possible ;).  That way errors due to missing modules are pretty obvious
>and don't depend on specific code being executed.
>
>

I like modularization and imports at the top of modules too.  But
there's a point where splitting one thing into lots of tiny modules
makes it harder to grok, especially when these classes have two-line
methods.  One module covering three pages is not an unwieldly unit, and
the class names give an idea what the dependencies are.  It also cuts
down on imports, which are also distracting.

How about Session and SessionManager in one module, and SessionStore and
*SessionStore in another?  That cuts the code approximately in half and
is a natural dividing line.

>-> - SQLSessionStore also assumes the connection is transaction safe.  DB-API
>-> says you cannot call conn.rollback() on an unsafe connection, yet the
>-> method just calls it anyway.  It works in MySQLdb (although you may get an
>-> "incomplete rollback" error sometimes).  I guess all the little SQL
>-> databases (SQL lite, Gadfly) are transaction safe so maybe it doesn't
>-> matter any more.  Do you really need to rollback at the beginning of each
>-> method?    I read somewhere that the application should give us a separate
>-> db connection unrelated to any other database operations in the
>-> application.
>
>You do need the rollback in any situation involving true transactions;
>otherwise multiple processes will not see the same committed data until
>after they do a commit.
>
>

I don't know what that means, but OK.  I thought rollback just threw
away any uncommitted changes.  What does that have to do with multiple
processes seeing the same committed data?