[Zope-dev] created z3c.saconfig

[Hermann Himmelbauer]

Am Donnerstag, 19. Juni 2008 20:51 schrieb Martijn Faassen:
> Hi there,
>
> I'd like to announce my contribution for the expanding list of options
> for SQLAlchemy integration for Zope 3.
>
> I've just implemented a package called z3c.saconfig which implements a
> utility-based way to set up SQLAlchemy's scoped session, as discussed
> recently on this.
>
> The package currently offers a way to configure SQLAlchemy sessions with
> a utility. This allows SQLAlchemy sessions to potentially differ per
> application. Currently if you would want to implement such utilities
> you're still on your own, but I intend to add support for this soon.

Yes, this looks very fine. I also find the documentation in README.txt 
explanative and well written. However, I have the following thoughts:

1) Why do you need to specify what interface the factory provides, such as 
here:

component.provideUtility(engine_factory, provides=IEngineFactory)
component.provideUtility(utility, provides=IScopedSession)

Why can't the utilities provide the interface out of the box?

2) I'd suggest to depict the case where an engine is bound to the session via 
the "bind=" parameter, as not all of us are that advanced in SA, thus it may 
be helpful. Moreover, you later on write that "setting up an engine factory 
is not actually necessary", so the reader may ask himself why the engine 
utility makes sense at all.

Perhaps it would be best to sketch the most simple case, with the bind 
parameter first, then explain what the shortcomings of this case are, and 
then introduce the engine utility.

3) I'd suggest to explain the part, where you do a "from z3c.saconfig import 
Session; session = Session()" a little, and line out that it's used in 
SQLAlchemy style, e.g.: "After registering the session utility, one can 
import the Session class vom z3c.saconfig, which offers the same capabilities 
as a common SQLAlchemy session class."

4) In the site examples, it reads:
sm1 = site1.getSiteManager()
sm1.registerUtility(engine_factory1, provided=IEngineFactory)

Why is it now "provided" instead of "provides"? Is this a typo or something 
specific?

5) For the siteScopeFunc part, it would be best if there would already be a 
generic one in the SiteScopedSession class, although I don't know if this 
would be possible. However, this would make things simpler for beginners. 
Later on I suggest to explain that it's possible to overwrite this method and 
what it's for.

The missing bits in this module seem to be:

1) Some way to update database parameters, e.g. change your engine: In many 
web applications, database setup is done by the user during installation 
(e.g. PHProjekt and many others). The user has some install wizard and inputs 
the database parameters here, moreover he can change them later on via a web 
frontend. I think there should be some solution/guideline that aids the 
programmer in this part. What I can think of is:

- Simply reregister the engine utility with new parameters
- Provide some "updateEngine" helper function, that queries a site/global 
registry for an IEngineUtility and alters the database parameters there
- Somthing like I suggested in my code, where there are specific configuration 
properties in the utility, that, if changed, recreate the engine.

However, it has to be take special care that when changing the engine, open 
sessions are not somehow corrupted.

2) Basically, I can think of 4 main scenarious for a Zope3 + SA integration:

a) 1 database per Zope3 instance
b) 1 database per Site
c) n databases per Zope3 instance
d) n databases per Zope3 Site

I suggest to outline that in the beginning of the README.txt along with some 
introductory words and explain that the setup for these cases differ. Maybe 
I'd include the case in the heading, such as 

GloballyScopedSession (for 1 DB per Zope3 instance)
=======================================

(a) and (b) seem to be most common and are covered in z3c.saconfig, but (c) 
and (d) seem to be missing. I don't know how common they are and if it makes 
sense to put a lot of effort into this. But maybe there's a solution via 
named utilities? Anyway, I'd outline that scenarious (c) and (d) are missing 
in this package, so that people know that right away.

All in all, thanks a lot for your efforts, this package will be very helpful 
to others who also need to integrate their application with a RDB!

Best Regards,
Hermann

-- 
hermann at qwer.tk
GPG key ID: 299893C7 (on keyservers)
FP: 0124 2584 8809 EF2A DBF9  4902 64B4 D16B 2998 93C7