Clojure-Contrib

Converted from http://www.assembla.com/spaces/clojure/tickets/50
Attachments:
c-c-sql-external-key.patch - https://www.assembla.com/spaces/clojure/documents/bsf2Y-5NCr3QaYeJe5afGb/download/bsf2Y-5NCr3QaYeJe5afGb
ticket-50-scg-1.diff - https://www.assembla.com/spaces/clojure/documents/d4S8n-6m8r3PaPeJe5aVNr/download/d4S8n-6m8r3PaPeJe5aVNr

scgilardi said: Hi Graham,

I recall now why I didn't have this as an option already. with-connection retrieves an open connection based on the info in db-spec, allows body to use it, then closes it. If we have the connection object in the db-spec object itself, it will only be good for one use.

How about this as an alternative to your patch (in addition to the other methods that will remain): Allow the user to provide a function (:factory) whose argument is a map of the remaining keys and values in the db-spec object. To retrieve a new, open connection, with-connection would call the factory object supplying a map of its params.

I think this will provide enough of an escape hatch for any source of open database connections to be used while still preserving db-spec as an immutable object that specifies how to retrieve them.

scgilardi said: [file:d4S8n-6m8r3PaPeJe5aVNr]

gmfawcett said: Hi Steve,

I had considered the :factory approach as an option, and would be fine with it – certainly it's a better general solution.

In my specific case, though, auto-closing is a good thing: Proxool connections are proxy-objects, and closing one signals that the underlying connection can be returned to the connection pool.

I think you're proposing adding logic to (with-connection*), to not use (with-open) if the :factory key is used? If so, perhaps we could also add a optional :factory-close key, whose value is either

• a cleanup function, e.g. {{{:factory-close (fn [conn] (.close conn))}}}
• a boolean, e.g. {:factory-close true} implies that (with-connection*) should use (with-open) to close the factory-supplied connection?

I'm happy to provide an alternate patch once we sort out the details. (I know it's only a few minutes of typing; no offense will be taken if you code something yourself and ignore my patch.)

gmfawcett said: Just to clarify my last comment: I understand your comment about not wanting a possibly-closed connection object as a value in the map. In my test code, the :external worked just fine – in spite of the "closed" object in the map, I could re-use it without any trouble. Proxool must be doing some magic to make this work. But this is bad form.

+1 for your :factory patch, and I'm still in favour of :factory-close if you would accept a patch for (with-connection*) as well.

scgilardi said: I like the uniformity of using with-open on all the connections. I'd like to save :factory-close until we have a usecase where it's needed. Could you please test the :factory patch in your setup? If you find that it works well for you, I'd like to commit it. Thanks!

gmfawcett said: I've tested your patch, and :factory works just fine for me.

I withdraw the :factory-close suggestion for now, since I don't need it. (I had suggested it based on a false assumption that you were patching with-connection* as well.)

Thank you!

importer said: (In revision:be33acd87f190d9ec2ad756d8cb31c88abca7e5f) Add Factory as a method of obtaining open database connections, fixes #50

Branch: master