Skip to content
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

Keeping init-key args around for a later halt-key! should be an implementation detail #25

Open
aroemers opened this issue Aug 24, 2017 · 6 comments

Comments

Projects
None yet
4 participants
@aroemers
Copy link

commented Aug 24, 2017

Hi @weavejester,

As discussed on Slack, we talked about how integrant's halt-key! method could have access to the original init-key arguments. It was the following example that spurred the discussion:

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

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

How to access the logger at the ??? in halt-key? A solution would be to have the following:

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

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

However, in my opinion this exposes an implementation detail (::datasource wanting to access the logger, so the actual datasource is inside a map) to the "consumers" of this component.

We discussed three possible solutions.

  1. Adapting halt-key's arguments to something like [_ pre-init-value post-init-value]. Upside is, that this is fairly straightforward. Downside is, it's not clear how to do this without breaking backward compatibility. The example however would be changed to the following:
(defmethod ig/halt-key! ::datasource
  [_ {:keys [logger]} datasource]
  (log logger :info ::closing.datasource)
  (.close datasource))
  1. Adding an Unref protocol having an unref function, together with some helper functions. Integrant will call unref if init-key's return value satisfies the Unref protocol, before passing it to other components. For example:
(defrecord Datasource [datasource logger]
  Unref
  (unref [_] datasource))

(defmethod ig/init-key ::datasource
  [_ {:keys [logger]}]
  (log logger :info ::starting.datasource)
  (map->Datasource
    {:datasource (hikari/make-datasource ...)
     :logger logger}))

Or, with some helper function:

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

Benefits of this approach are backward compatibility and the developer can keep even more values than just the pre-init value.

  1. A workaround for now without changing integrant, by using the ::build metadata on a system.

As I also stated on Slack, I am just getting started with Integrant, so we agreed we would both ponder if this is an actual problem, and if so, what the best approach would be to solve it.

@martinklepsch

This comment has been minimized.

Copy link

commented Aug 26, 2017

Just for posterity: I also bumped into this limitation. It's not too hard to work around but certainly something people are used to as being easy coming from component.

@weavejester

This comment has been minimized.

Copy link
Owner

commented Aug 26, 2017

Thanks for writing up the conversation @aroemers!

@weavejester

This comment has been minimized.

Copy link
Owner

commented Aug 26, 2017

It's not too hard to work around but certainly something people are used to as being easy coming from component.

What do you mean? Component has the same problem, but worse, in that any dependancy with a lifecycle needs to be wrapped in a record.

@martinklepsch

This comment has been minimized.

Copy link

commented Aug 26, 2017

I mean in Component I have access to all values that I had access to when start was called whereas with integrant I only have access to all values that my init-key returns.

I guess the situation in Component would be similar if you would be allowed to return anything else than the record being started...

@weavejester

This comment has been minimized.

Copy link
Owner

commented Aug 26, 2017

I mean in Component I have access to all values that I had access to when start was called whereas with integrant I only have access to all values that my init-key returns.

In Component, stop only has access to the data that start returns, the same as Integrant, where halt-key! only has access to the data that init-key returns.

@neverfox

This comment has been minimized.

Copy link

commented Aug 27, 2017

@weavejester I think @martinklepsch is referring the the fact that, as a consequence of using records, you can have access to the original configuration bindings in all protocol methods no matter what you return from start. For example:

(defrecord Datasource [logger] ;; by referencing logger here, you can close over it
  component/Lifecycle
  (start [_]
    (log logger :info ::starting.datasource)
    (hikari/make-datasource ...))
  (stop [self]
    (log logger :info ::stopping.datasource)
    (.close self))

That said, I'm so used to writing my components in record-based frameworks by assoc-ing on the original config map (like @aroemers's second example) rather than returning something novel, that I never really thought of this as an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.