korma / Korma

Tasty SQL for Clojure.
http://sqlkorma.com
1.48k stars 222 forks source link

Adds a helper for transactions with a specific isolation level #246

Closed ls4f closed 9 years ago

ls4f commented 9 years ago

Now that Korma has moved to the new JDBC, I figure it should have a way of specifying the desired isolation level (without ConnectionCustomizerClassName which I had suggested in a previous pull request).

immoh commented 9 years ago

While I think the functionality would be useful to have, I’m not really comfortable with introducing new macro for it. From API perspective it would be best to give isolation level and read-only flag as options to transaction macro.

ls4f commented 9 years ago

I couldn't think of a nice way to alter the transaction macro and preserve behavior at the same time. If you have an idea, I'm willing to rework it.

immoh commented 9 years ago

API could look like this:

(transaction {:isolation-level :serializable :read-ony? false}
  (insert ..)
  (delete ..))

also supporting

(transaction
  (insert ..)
  (delete ..))
ls4f commented 9 years ago

I figured as much. What I meant was that I'm not terribly sure if I can get the macro right. I believe now it's ok, meaning it recognizes the map and it should not break existing code. It does however look uglier to me that way, and I can't think of a better way to do it at the moment. I think the read-only? flag was pushed in clojure-jdbc 0.3.5 so I bumped the version.

If you think it's ok, I'll squash the commits.

immoh commented 9 years ago

Looks good. Some things I would still like to see:

ls4f commented 9 years ago

Ok

immoh commented 9 years ago

I think the test is ok. Please squash commits and rebase on latest master, I'll merge it then.

ls4f commented 9 years ago

Done

immoh commented 9 years ago

Thanks!