bitemyapp / revise

RethinkDB client for Clojure
146 stars 8 forks source link

Add ability to specify default db #9

Open cesarbp opened 10 years ago

cesarbp commented 10 years ago

This is possible in the other drivers, no reason it shouldn't be possible here.

As long as there is no need for global state.

bitemyapp commented 10 years ago

I think the best approach here is partial application of the run/run-async functions by the user.

(def myrun (fn [query] (run query my-conn))
danielytics commented 10 years ago

:+1:

It took me a little while to realise that I had to use (-> (r/db :foo) (r/table-db :table) ... (run conn)) when all of the example code in the readme does (-> (r/table :table) ... (run conn)).

I notice the official rethinkdb drivers have a db option for connect.

john2x commented 10 years ago

Would something like (-> (r/db :foo) (r/table :table) ... (run conn)) be possible? Or maybe include the db name when creating the conn?

danielytics commented 10 years ago

I don't think its worth it just to drop 3 characters -db from each query. Allowing a database to be set per connection saves the call to r/db too.

Besides adding a db option to connect, which would definitely be my preferred solution (the general case is that people use a single database with a connection, after all), another option is something like this:

(with-db :foo
  (-> (r/table :table) ... (run conn)))
cesarbp commented 10 years ago

The only function that takes a db IIRC is table-db. So you can easily just use clojure to set a "default" database like so:

(defn my-db-table
  [table-name]
  (-> (r/db my-db) (r/table-db table-name)))

This is probably the simplest way.

danielytics commented 10 years ago

Would be nice not to have to make database-specific functions like that, but it certainly does the job - and you're right, its definitely the simplest "fix".

sritchie commented 10 years ago

Clutch has a "defdbop" macro that allows you to optionally declare the database as the first option of the defn:

(defmacro defdbop
  "Same as defn, but wraps the defined function in another that transparently
   allows for dynamic or explicit application of database configuration as well
   as implicit coercion of the first `db` argument to a URL instance."
  [name & body]
  `(do (let [ret# (defn ~name ~@body)]
         (alter-var-root (var ~name) @#'c/with-db*)
         (alter-meta!
          (var ~name)
          update-in [:doc] str
          "\n\n  When used within the dynamic scope of `with-db`, the initial `db`"
          "\n  argument is automatically provided.")
         ret#)))

Might be helpful here. Not sure if you guys like the pattern.

bitemyapp commented 10 years ago

@sritchie entirely up to @cesarbp but I was scarred by implicit db arguments after Korma and other Clojure libraries.

It's a good pattern to keep in mind, my preference would be a lexically scoped "with-revise-db" macro rather than something quite that scary. I don't know where @cesarbp 's priorities are at this time.