Clojurians Log v2

I'm just getting started with integrant, so this may be a silly question. Say I have the following:

(defmethod ig/init-key ::datasource
  [_ {:keys [config logger]}]
  (log/info logger "Starting datasource...")
  (hikari/make-datasource ...))

(defmethod ig/halt-key! ::datasource
  [_ datasource]
  (log/info ??? "Closing datasource...")
  (.close datasource))

I think this is the example similar to your needs: https://github.com/duct-framework/server.http.jetty/blob/99e4cb0c87f587ff38525f8547b2a61a993f4831/src/duct/server/http/jetty.clj I don’t know if it is the only option though.

@arnout @jahson Since that code is written by the guy who write integrant, it's probably a pretty idiomatic way to do it :)

Hmm, using a map indeed..

Thanks for the example

Maybe halt-key! could/should take 3 arguments: the key, the referenced values and its own value?

As far as I understand integrant is a way to transform a config to a running system. Special handling of referenced values will probably add unnecessary complexity. I think @weavejester will explain much better.

Thing is, every other component that uses the datasource component, must now know that it has to fetch it from a map using some key, instead of using it directly. This use of maps (or records) is a bit like Stuart's Component library. How would repeating the values you receive at init-key in the halt-key! function add unnecessary complexity?

https://github.com/weavejester/integrant#configurations There is almost no need for a component to know a key, you can use #ig/ref to reference component you need. You still need to know the key to get it from init-key second argument, but is more like public contract to me.

The keys in config will be replaced with results of running each init-key, so when the halt-key! is running it is operating not on config, but on system.

With this I agree > You still need to know the key to get it from init-key second argument, but is more like public contract to me. But, in case of the datasource example above, the other components need to know two keys: its own reference (which is fine indeed), and an extra, more implementation specific key to actually get to the datasource object.

> The keys in config will be replaced with results of running each init-key, so when the halt-key! is running it is operating not on config, but on system. True, but I don't have access to that system in halt-key!, exactly my "issue".

The system has some meta associated with it though.

And probably integrant can provide refs as the third argument for example, but for some reasons it has gone other way. I think the author can explain why.

> I think the author can explain why. Sure, when @weavejester comes online I am curious about his answer. Thank you anyway for your insights, @jahson!

AFAIK you always need some kind of extra to do IOC, be it some @Annotation or interface or other things.

Also you can use the most generic key, but provide your own specific implementation.

https://github.com/weavejester/integrant#derived-keywords

Sure, the extra thing is the #ref or (ig/ref ...) in the case of integrant.

Yeah, it helps with decision of what to init first.

@arnout Thanks for the question. When it comes to data sources, my approach is to always wrap them in a “boundary” record:

(defrecord Boundary [datasource])

(defmethod ig/init-key ::datasource [_ {:keys [config logger]}]
  (log/info logger "Starting datasource...")
  (map->Boundary {:datasource (hikari/make-datasource ...), :logger logger}))

(defmethod ig/halt-key! ::datasource
  [_ {:keys [datasource logger]}]
  (log/info logger "Closing datasource...")
  (.close datasource))

This allows for a layer of indirection through protocols:

(defprotocol Users
  (get-user [db user-id]))

(extend-protocol Users
  Boundary
  (get-user [{:keys [datasource]} user-id] ...))

This allows for faster testing via database mocks, as functions that take the datasource can now take something that adheres to specific protocols instead.

Sure, this much I understand. But let's say, I don't want to wrap the component (whatever it is, a datasource, a logger, something else) in a record or map. Because I don't have access to the system on halt-key!, I am still forced to use a record or map.

Right.

I guess we could add in something extra that would carry across extra data without affecting the ref, but it would add complexity.

Effectively you’d need an boundary that’s automatically unwrapped.

Or allow 3 arguments to halt-key!? I've looked at your source, most ingredients are there; the build function does most of the work, but is only used on init.

@arnout But you’d need two return values from init-key - the bit you want to use as a reference, and the bit you want to pass to halt-key!.

Unless you just use the input to init-key.

Same input as init-key would be nice I think? E.g. (defmethod halt-key! ::my-thingy [_ pre-init-value post-init-value]).

(or swap those two arguments, for backwards compatibility)

Or did you mean something else?

I believe I keep the “pre-init-value” around through ::build metadata for use in resume-key, so that would be possible.

Another solution would be to supply a protocol that “unwraps” the value we want to reference from any extra data.

(defprotocol Unref
  (unref [x]))

(defrecord DataSource [datasource logger]
  Unref
  (unref [_] datasource))

(defmethod ig/init-key ::datasource [_ {:keys [config logger]}]
  (log/info logger "Starting datasource...")
  (->DataSource (hikari/make-datasource ...) logger}))

A protocol would be more flexible, as we could keep extra information not found in the pre-init-value.

And it would also be backward compatible.

Though somewhat harder to use, unless we wrote a helper function:

(defmethod ig/init-key ::datasource [_ {:keys [config logger]}]
  (log/info logger "Starting datasource...")
  (unref (hikari/make-datasource ...) {:logger logger}))

(defmethod ig/halt-key! ::datasource [_ {:keys [ref logger]}]
  (log/info logger "Stopping datasource...")
  (.close ref))

Interesting indeed

I could use the ::build metadata for now, without changing integrant source, which gives us some time to reflect on this? It's nice to know that there are options, and it seems you acknowledge the value of such an addition?

I don’t think the ::build metadata is directly accessible from halt-key, because it’s on the configuration. However, you could use reverse-run! to execute your own function.

I’ll need to think about this some more, but I tentatively acknowledge the value of it. My thought is that you might want to pass information to halt-key! that is not in the pre-init-value, so I tentatively favour introducing a protocol.

But I think I’d need to see more use-cases to get a feel for what’s the right option.

I was thinking in the lines of:

(def ^:dynamic *build*)

(defn my-halt [system]
  (binding [*build* (::build system)]
    (ig/halt! system)))

(defn pre-init-value [key]
  (get *build* key))

(defmethod ig/halt-key! ::my-thingy
  [_ value]
  (let [{:keys [logger]} (pre-init-value ::my-thingy)]
    ...))

Ah, yes, that would work.

Ok, let me know if you have some news regarding this. Or would you like an issue submitted about this at GitHub?

@arnout Please. It’ll mean I don’t forget about it 🙂

It might be a while before I get around to implementing this. I’m trying to be careful with what I add to Integrant, as it’s hard to take away things later on.

And right now I can’t think of a use-case for this where I wouldn’t use a boundary record anyway.

Ok, I will submit an issue.

Regarding a use-case, I'm all for using Boundary records, though as per your own recommendation a better database related boundary would be the following?

(defprotocol Users
  (new-user [this username])
  (get-user [this username]))

(defrecord UsersImpl [datasource]
  Users
  (new-user [_ username]
    (... use datasource ...))
  ...)

@arnout Typically you only want one record per data source, because you can write multiple protocols against it.

So in Duct, I have a duct.database.sql.Boundary record for all SQL databases.

That way I can switch connection pools transparently.

The idea is to wrap any I/O in a protocol, so that it can be substituted for a fake, mock, stub or even another valid data source.

It will look like this

(ns example.boundary.account-options
  (:require [yesql.core :as yesql]
            [duct.database.sql]))

(yesql/defqueries "example/sql/account-options.sql")

(defprotocol AccountOptions
  (find-account-options [this acc-id]))

(extend-protocol AccountOptions
  duct.database.sql.Boundary

  (find-account-options [{:keys [spec]} acc-id]
    (->> {:connection spec}
         (find-by-acc-id {:acc_id acc-id})
         first)))