Closed k13gomez closed 9 years ago
Requiring users to implement multimethod for creating datasource seems a bit awkward when they could simply pass it to defdb/createdb, e.g.:
(defdb mydb (postgres {:datasource (HikariDataSource.)})
Users are anyway required to configure the datasource (including driver class and jdbc url) by themselves so I guess Korma can only help in getting the generated sql right with options from db helper functions such as postgres. This doesn't work yet but maybe we should look into getting it working instead of introducing the multimethod?
Could resolve-new be simplified to this?
(defmacro resolve-new [class]
(when-let [resolved (resolve class)]
`(new ~resolved)))
Good catch, it would indeed be more efficient to leave it up to the user to make the pool. I removed the multi-method; take a look at this approach which would allow combining a spec and a custom pool fn:
(defdb mydb
(postgres {:db "korma" :make-pool? my-custom-pool-fn}))
Let me know if you like this approach.
Also, see a possible integration approach here, for HikariCP: https://github.com/k13labs/korma.hikari-cp
This is no different from the multimethod solution. What is the benefit over just taking the datasource object in (which already works)? Other than users getting classname and jdbc url from Korma?
Also I don't like reusing boolean key :make-pool? for custom functions.
I reverted back to the simpler create-db, keeping only the logic to skip calling connection-pool when c3p0-enabled? is false, custom pool functions could still use the classname and jdbcurl provided by korma like this:
(defdb mydb
(my-connection-pool-fn (h2 {:db "mem:sample1"})))
It should throw an exception if make-pool? is true (as it is by default) but c3p0 has been excluded from dependencies. If users don't want Korma to create connection pool they should always set make-pool? to false and optionally exclude c3p0 from deps.
I threw out that last commit which changed the create-db logic, an exception is now thrown if make-pool? is true but c3p0-enabled? is false; should be as simple as can be now.
Thanks!
For those who want to integrate HiKariCP as connect pool, there is copy & paste snippets for mysql:
(ns demo.db
(:require [korma.db :refer :all]
[hikari-cp.core :refer [make-datasource]]
[korma.core :refer :all]))
(def ds (make-datasource {:pool-name "test-pool"
:adapter "mysql"
:username "root"
:password "123"
:database-name "korma"
:server-name "127.0.0.1"
:connection-test-query "select 0"}))
(defdb pool-db {:datasource ds
:delimiters "`"})
HiKariCP options can be found here.
Also, I suggest add some steps in README for how to integrate third-party datasource. @immoh
accomplished through the connection-pool multimethod.
fallback to non-pooled mode if c3p0-enabled? is not truthy
See #224 and @immoh 's comment on #127.
This allows exclusion of c3p0 based on the user's needs, and the connection pool is resolved similarly to clj-http. Also, it opens Korma to extension via connection-pool multimethod.
I'll implement hikari-cp support as a separate library that can be included if desired.