This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2020-07-31
Channels
- # announcements (2)
- # babashka (145)
- # beginners (260)
- # calva (17)
- # chlorine-clover (7)
- # clj-kondo (9)
- # cljsrn (1)
- # clojure (88)
- # clojure-dev (65)
- # clojure-europe (31)
- # clojure-france (4)
- # clojure-nl (4)
- # clojure-uk (61)
- # clojuredesign-podcast (1)
- # clojurescript (31)
- # code-reviews (1)
- # cursive (32)
- # data-science (2)
- # datascript (9)
- # datomic (39)
- # docker (3)
- # events (1)
- # figwheel (95)
- # figwheel-main (4)
- # fulcro (17)
- # kaocha (2)
- # keechma (1)
- # malli (1)
- # meander (35)
- # nrepl (4)
- # off-topic (1)
- # pathom (8)
- # re-frame (4)
- # reagent (8)
- # reitit (3)
- # releases (1)
- # remote-jobs (2)
- # shadow-cljs (182)
- # sql (30)
- # tools-deps (89)
- # xtdb (31)
@seancorfield For my use case it would be very helpful to allow to call get-connection with username and password in next-jdbc. I would like to propose a patch but am unsure if you would prefer to have username and pw as part of opts or as new arities of get-connection.
@hannes948 Is there a reason for you to not provide :user
and :password
as part of the hash map passed to get-datasource
?
@seancorfield Sure, that's possible. But I am using different users with the same datasource and thought this approach would be cleaner and could help others as well.
The DataSource
object supports (.getConnection ds username password)
so you could get your connections that way.
(and that's true of the reified DataSource
that get-datasource
produces from a hash map or URL)
Oh yeah. I was already using get-connection. I see, my patch would just be aesthetics then. 😉
I understand that's not as convenient so I'll have a think about it. Feel free to create an issue on GitHub.
If you use a connection pooled datasource, aren't you restricted to a single user/password (since you have to provide that to c3p0 or HikariCP)? Or do those support connection-level user/password settings?
Given that I can't change the Connectable
protocol without (potentially) breaking existing code, I'd want to pass :user
/ :password
via the opts
there (even if next.jdbc/get-connection
was updated to have extra arities, it would resolve to (p/get-connection spec opts)
under the hood), so then I think the only change would be this line in the private make-connection
function would need to check for those keys in the opts
:
(let [^Connection connection (.getConnection datasource)]
would become something like
(let [^Connection connection (if (and (:user opts) (:password opts)) (.getConnection datasource (:user opts) (:password opts)) (.getConnection datasource))]
^ @hannes948 Thoughts?@seancorfield hikari will throw an Unsupported SQL Exception, c3p0 has different pools for username+password
(for the reified DataSource
, that arity .getConnection
only folds the username and password back into the etc
hash map under the hood when calling DriverManager/getConnection
anyway)
I am fine with your proposal above, but my first attempt was to add new arities to the functions. We would go from spec
and spec opts
to spec user pass
and spec user pass opts
. I am not very experienced with java/clojure backwards "ABI" compatibility, but this change could break external users of the protocols, right?
Right. I'm not changing the arities in the protocol.
I was writing a bit lengthy github issue talking about exactly this but do we want to continue here and I just submit a basic feature request?
I'm open to changing the arities in the top-level wrapper (`next.jdbc/get-connection`) but it would just assoc
those into opts
and pass that down. I'm just not sure of the value of that, given the narrowness of the use case (either unpooled connection or just c3p0, based on your comments above).
The change is pretty minor so I might as well go ahead and make it anyway at that level.
I would welcome that! It allows me to continue to use the other options as well that get-connection provides and not having to fall back to a custom wrapper.
@hannes948 Fixed on develop. Let me know how that works for you. I'll probably cut a new next.jdbc
release this weekend if you need a published version on clojars?
@hannes948 awesome, I can quickly pull it in and give it a test. Will report back in few minutes.
There's already the execute-batch!
enhancement pending a release so I was planning an update soon anyway.
having mixed auth in one pool sounds like a security problem
I remember reading that c3p0 supported that and thinking "Hmm, I wonder why anyone would do that?" 🙂
@noisesmith they are distinct pools in c3p0, they are hashed by user+pw, the other pools that don't support it just throw an exception
@hannes948 I can understand that. We have separate datasources for r/o vs r/w so it's usually very clear in our code which one is being used where.