-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
a way to get a map of functions #19
Comments
I think I'm open to adding a function or macro to accomplish this. My personal preference on database-calling functions is to pass in the db-spec/connection/pool/transaction object as the first parameter. However, the fact that quite a few folks may be using Luminus/conman and would be familiar with its usage pattern definitely lends some weight to this. Let me sleep on it! I'm marking this as a future enhancement. |
Sounds great. The way I set it up in conman is that you can either use the connection value that the functions have been bound to or pass a connection explicitly. Exposing a map of functions to the user opens up a lot of possibilities. For example, you could do things like: (def queries (parse-queries "queires.sql"))
(defn get-users [conn query-params]
((get-in queries [:get-users :fn]) conn query-params)) This gives the users the option to do things explicitly as well as having the library define the functions for them. |
@yogthos I slept on it, then added some macros for this:
These functions return a hashmap of the form: {:fn1-name {:meta {:doc \"doc string\"} :fn <fn1>}
:fn2-name {:meta {:doc \"doc string\" :private true} :fn <fn2>}} I've started a 0.4.1-SNAPSHOT in master. Would you be willing to clone the repo and do a local install to ensure this will work for your needs? Since hugsql is a meta-project, you can use the alias |
Also, something to consider when wrapping up functions is the original arities of the functions produced by HugSQL. From the docs: The functions defined by def-db-fns have the following arities:
where:
|
Great, I can definitely help with test driving this. Reading the code, I don't think the current approach will quite work for my use case though. The use case I have is that I wish to avoid implicitly defining the functions by HugSQL and have the I propose splitting out the code in Let me know if I'm making sense here. :) |
In the just-committed code with Does that make sense? |
Ah ok I think so, I misread the last bit of |
Great! Let me know if you run into any issues. I'm happy to answer any questions to get this right. |
Just for clarification on expression compilation: All of the
|
I've got the latest compiled locally, and so far everything's working exactly as expected. Thanks on the quick turnaround on this by the way. |
So I thought of an issue you might run into and I think I have a fix for it. Since conman will be acting in a "supervisory" role to define hugsql functions, you'll have to account for the difference between a database function and a snippet function. Snippet functions do not take database connections, so you will not want to define them as such. A snippet function's arities are: [sql]
[sql options] So, in the hashmap you receive from Does that make sense? |
Ah yes, good catch, this way I'll can simply filter on that. |
So, this seems to be working as expected, does it look good to you? (defn load-queries [filenames]
(reduce
(fn [queries file]
(->> (hugsql/map-of-db-fns file)
(remove #(get-in % [:meta :snip?]))
(into queries)))
{}
filenames))
(defmacro bind-connection [conn & filenames]
`(let [queries# (load-queries '~filenames)]
(doseq [[id# {fn# :fn}] queries#]
(intern *ns* (symbol (name id#))
(fn
([] (fn# ~conn {}))
([params#] (fn# ~conn params#))
([conn# params#] (fn# conn# params#))
([conn# params# opts# & command-opts#]
(apply fn# conn# params# opts# command-opts#))))))) |
I think users would expect the snippets to actually be defined also, since they would be using the snippets in the same namespace. |
Ah right, so perhaps something like this: (defn load-queries [filenames]
(reduce
(fn [queries file]
(let [{snips true
fns false}
(group-by
#(-> % second :snip? boolean)
(hugsql/map-of-db-fns file))]
(-> queries
(update :snips into snips)
(update :fns into fns))))
{}
filenames))
(defmacro bind-connection [conn & filenames]
`(let [{snips# :snips fns# :fns} (load-queries '~filenames)]
(doseq [[id# {fn# :fn}] snips#]
(intern *ns* (symbol (name id#)) fn#))
(doseq [[id# {fn# :fn}] fns#]
(intern *ns* (symbol (name id#))
(fn
([] (fn# ~conn {}))
([params#] (fn# ~conn params#))
([conn# params#] (fn# conn# params#))
([conn# params# opts# & command-opts#]
(apply fn# conn# params# opts# command-opts#))))))) |
Yes, something like that will get users a consistent experience across both conman and hugsql. If you're fairly confident that these additions to hugsql will work, I can push out a 0.4.1 release a bit later today. Let me know. Thanks! |
Thanks, so far I haven't run into any problems. I think it's probably safe to push this out. :) |
This is released in 0.4.1 |
Awesome! I'm just cleaning up conman, and probably going to push out a new version of it today as well. :) |
probably worth noting this in the Yesql comparison section http://www.hugsql.org/#faq-yesql |
And pushed out new conman that's backed by HugSQL now, I'll have to do some updates for Luminus, mostly in the documentation and then I'll switch it over to use the latest conman. |
Very cool! Since HugSQL is not an exact drop-in for Yesql, you may need some kind of "converting Yesql to HugSQL" doc to aid existing users. I'd be happy to answer any questions along those lines when the time comes. |
Sounds great, I'll probably add this in the changelog for Luminus. The nice part is that for most Yesql queries the transition should be very straight forward. The timing on this worked out very well for me, I'm just wrapping up the beta for the Clojure web dev book I'm publishing with pragprog, and I'm going to update it to reflect the change. Would've been unfortunate if it got published right before I switched Luminus off Yesql. So, in other news HugSQL will get a bit of promotion there as well. :) |
Sounds great! What good timing! I really appreciate all of the work you've put forward in the Clojure web development space in libraries, documentation, and the book. Great stuff! |
Thanks, I really appreciate it. It's been great seeing the ecosystem grow, and to be able to work with great libraries such as yours. |
I'm currently using Yesql in Luminus, but I'd like to either switch to HugSQL or support it as an alternative. Currently, I manage the connection using the conman wrapper library I wrote.
One of the things I wanted to do was to ensure that the transactional connection was used by default. I find that the behavior of having to remember to pass it in is quite error prone, and you only get an error in case the transaction would fail.
My approach was to use a dynamic var store the connection, and then rebind it within transactions. What I ended up doing with Yesql is creating a shadow namespace where it declares its functions and then wrapping those with functions I generate. he functions defined by conman use the dynamic var and get the transactional connection by default.
It would be really nice to have the ability to get a map of anonymous functions keyed on their names from HugSQL that could then be used to declare my own functions in conman.
If something like this was available, that would be great :)
The text was updated successfully, but these errors were encountered: