search

metabase / toucan

A classy high-level Clojure library for defining application models and retrieving them from a DB
Eclipse Public License 1.0
570 stars 49 forks source link

Transactions do not work with *db-connection* #42

Closed miikka closed 5 years ago

miikka commented 5 years ago

The way I'm using Toucan is that I never call set-default-db-connection! and instead I always set *db-connection*. I'm using Integrant and explicitly passing the database connection fits that model better than having global state.

I just noticed that transactions do not work with *db-connection* – if you use toucan.db/transaction, the code inside the macro will not be run inside a transaction. This is because toucan.db/connection always returns *db-connection* if it's set: https://github.com/metabase/toucan/blob/c5567c8315b004f7565ff58e55f22005a39a5de8/src/toucan/db.clj#L124-L127

I'm not sure if this is a bug or an intentional feature, but I certainly expected transactions to work. As a workaround, I'm now binding toucan.db/*transaction-connection* (which is marked as private) instead of *db-connection* and that works fine.

miikka commented 5 years ago

I thought about it more and concluded that the current behavior is a feature: it's good to be always able to tell the database functions which connection they should use and *db-connection* allows you to do exactly that.

Binding *transaction-connection* supports my use case well even if the name is a bit misleading. Would it make sense to make this use case more "official" by marking *transaction-connection* as public?

@camsaul

camsaul commented 5 years ago

Hey @miikka that's a use case I really haven't thought much about. If you want to go ahead and make *transaction-connection* public I think that's the best course of action here.