korma / Korma

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

Get rid of global options atom (korma.config) #304

Closed immoh closed 9 years ago

immoh commented 9 years ago

Having global options doesn't make much sense, as options can be defined on db level and there is a global default db. Options are really database specific, having global options only makes things harder to implement correctly (e.g. #282)

This is a breaking change.

Most likely will be a bit tricky to implement because korma.sql.engine relies on fallback to korma.config/options.

See more discussion in #299.

k13gomez commented 9 years ago

take a look at this approach on #306, I removed the korma.config/options global atom and used the new get-options function throughout the stack which relies on a combination of dynamic binding vars, query/db arguments, or fallback to the get-defaults multimethod.

Using a multimethod would allow someone using the API to override the defaults for a given subprotocol based on their needs (there's a test written to demonstrate this approach). We could also pre-package different best-practice defaults for the different subprotocols by defining multimethods accordingly.

k13gomez commented 9 years ago

See my latest commit, should be closer to what you're looking for.

gfZeng commented 8 years ago

IMO, global config options is necessary. And korma.db/->name should definition as:

;;; korma/db.clj
(defn- ->naming [strategy]
  (merge (:namimg options) strategy))
immoh commented 8 years ago

I would like to hear why.

gfZeng commented 8 years ago

@immoh

(require '[korma.config :as kconf])

;;; set default naming
(kconf/set-naming {:keys underscore->dash :fields dash->underscore})

(db/defdb pg (db/postgres DB-SPEC))
;;; now query will use default naming strategy
immoh commented 8 years ago

It doesn't explain why it should be global. In my opinion the naming strategy is related to how the used database expects entities (tables, fields) to be named (uppercase, underscore etc.) hence it is related to database connection and should be defined in the spec.

gfZeng commented 8 years ago

Naming strategy is a transform for field names. which can affect input and output. It should be careless by api users.

the db-spec should shared with every api users. well, Naming strategy should can be customized. API provider just provider default options.