weavejester / integrant

Micro-framework for data-driven architecture
MIT License
1.22k stars 63 forks source link

Would you consider an integrant2 to support extensible build steps? #82

Closed codestiff closed 4 years ago

codestiff commented 4 years ago

I'm debating hosting my own project for this. I would like to reuse the work you have here. Would you be interested in an integrant2 api or a way to make a backwards compat API?

https://github.com/codestiff/fashionable/blob/master/README.md

Example:

(defmethod fashion :initialize ::node
  [_ config]
  ; code which creates stateful java object
  (builders/build-node config))

(defmethod fashion :initialize ::node-discovery
  [_ config]
  ; code which creates stateful java object
  (builders/build-node-discovery config))

(def config
   {::node
         ;; become java: Node objects
         ;; can be looked up in the system map via: (get-in system [::node :a]) or :b or :c)
          {:a {:id "node-a" :address "localhost:5767"}
                 :b {:id "node-b" :address "localhost:5768"}
                  :c {:id "node-c" :address "localhost:5769"}
            ;; here we can see a reference to collection of nodes
            ;; can be looked up in the system map via: (get-in system [::node :base-nodes])
            :base-nodes [
                         (->Ref [::node :a])
                         (->Ref [::node :b])
                         (->Ref [::node :c])
                         ]
            }
    ::node-discovery {:default {:nodes (->Ref [::node ::base-nodes])}}
    }
   {::node-discovery [:default]})

(fashionable/up config [:prep :initialize :start]) => {:system {::node-discovery {:default #NodeDiscovery { .. }}}
(fashionable/down config [:prep :initialize :start])

The point in the example, is that instead of only 2 steps prep and init which is how integrant currently works. There would be a default of 2 steps prep and init but a user could define additional steps (as they see fit). In the example, the user's steps are: [:prep :initialize :start]

weavejester commented 4 years ago

Integrant is currently only version 0.8.0, so it might be a little early to think about Integrant 2.

I'm afraid I don't understand what you mean by "steps" here. What's the difference between a step and a function that transforms the configuration or system map?

Also, are you aware of the build, run! and reverse-run! functions?

codestiff commented 4 years ago

Integrant is currently only version 0.8.0, so it might be a little early to think about Integrant 2.

I more mean that this design proposal is probably not backwards compat (Although, in theory it could be...). So it could go in a different integrant namespace.

I'm afraid I don't understand what you mean by "steps" here. What's the difference between a step and a function that transforms the configuration or system map?

Depends what you mean as well but in my head steps are chained together. The result of your previous step is passed to the next build step of the component. In practice, someone can combine any number of steps into a single step which does it all, but my thought is that breaking it apart makes it easier to reason about.

A small example, let's pretend we have 3 steps: [:load-defaults, :build-objects, :run] Let's suppose we are configuring a http server to bind to and address on our machine and it includes a third party worker pool library.

(def config (read "my-file-location.edn"))

;; let's suppose config looks like
{:my.server/default {:addr "localhost",
                     :worker #ref [:my.worker/default],
                     :heartbeat #ref [:my.heatbeat.clientpool/default]},
 :my.worker.default {:heartbeat #ref [:my.heatbeat.clientpool/default]},
 :my.heartbeat.clientpool/default {:addr "https://internal.heartbeats.not.real.example"},

;; after the :load-defaults
;; it would look like
;; notice we added a default port
;; and max-threads
{:my.server/default {:addr "localhost",
                     :port 8080,
                     :worker #ref [:my.worker/default],
                     :heartbeat #ref [:my.heatbeat.clientpool/default]},
 :my.worker.default {:max-threads 8,
                     :heartbeat #ref [:my.heatbeat.clientpool/default]},
 :my.heartbeat.clientpool/default {:addr "https://internal.heartbeats.not.real.example",
                     :max-threads 4}}

;; after :build-objects
;; it would look like
;; we converted our config data
;; to java objects which will run
;; our program
{:my.server/default #object [some.jetty.server 0xdeadbeef ...],
 :my.worker/default #object [thirdparty.worker.library 0xdeadbeef ...],
 :my.heartbeat.clientpool/default #object [thirdparty.heartbeat.client 0xdeadbeef ...]}

;; after the :run step
;; the map would look the same
;; but the step function :run
;; would be called for both
;; :my.server/default and :my.worker/default

;; sidenote, it would be useful to get the config of one of the components
;; example
(fashionable/input-of system :build-objects [:my.server/default]) =>
{:my.server/default {:addr "localhost",
                     :port 8080,
                     :worker #object [thirdparty.worker.library 0xdeadbeef ...],
                     :heartbeat #object [thirdparty.heartbeat.client 0xdeadbeef ...]}}

In think you can accomplish the above example, using integrant today using: prep and init but I think there is value in being able to customize and break apart those steps.

Also, are you aware of the build, run! and reverse-run! functions?

Let me take a look. I think integrant works well for most cases but on more than one occasion I had the desire to separate out different build steps. Which I think is the main proposal (being able to extend/customize the build steps), not that you can't accomplish the same goal w/ integrant as it is now.

codestiff commented 4 years ago

Maybe, combining something like aero and integrant as it is now is good enough.

weavejester commented 4 years ago

Depends what you mean as well but in my head steps are chained together. The result of your previous step is passed to the next build step of the component.

Can't you just chain the functions? e.g.

(-> config foo ig/prep bar ig/init baz)

Where foo, bar and baz are your custom steps.

You can use the build, run! and reverse-run! functions to create "step" functions with different dependency resolutions. Behind the scenes, init uses build, and halt! uses reverse-run!

codestiff commented 4 years ago

I will take take a look at build and run!

(-> config foo ig/prep bar ig/init baz)

That's a good example. I imagine creating foo, bar, and baz would miss out on the key resolution configuration that ig/prep and ig/init is aware of.

Example, let's suppose foo needs to add a default port for the keys:

Example 1: I would imagine the definition of foo would look something like:

(defn foo
  [config]
  (reduce
    (fn [c [k defaults]
      (update c k merge defaults)
    config
   [[:my.database/primary {:port 4336}]
    [:my.database/backup {:port 4337}]])))

Example 2: Another example (if the developer is feeling more ambitious) is to mimic how prep or init work:

(defmulti prep-foo (fn [k config] k))

(defmethod prep-foo :default [k config] config)

(defmethod prep-foo :database/postgresql [k config]
  (let [defaults {:my.database/primary {:port 4336}
                     :my.database/backup {:port 4337}}
  (merge defaults config))

(defn foo
  [config]
  (reduce
    (fn [c k]
      (prep-foo k c))
    config
    (keys config)))

I would prefer to be able to leverage the existing resolution code in integrant and only have to write:

;; let's assume I defined the original method :database/postrgresql
;; if someone else defined it, we can create a new keyword (e.g. :my.database/postgresql)
;; and have :my.database/primary or :my.database/default derive
;; from the new keyword instead
(defmethod prep :foo :database/postgresql [k config]
  (let [defaults {:my.database/primary {:port 4336}
                     :my.database/backup {:port 4337}}
  (merge defaults config))

I would prefer not to do Example 1 because it is aware of each key (across different components) that it is updating. And I would also not to do Example 2 because it seems like I'm reimplementing something which has already been solved in order to make foo work.

Maybe run! and build solve this.

codestiff commented 4 years ago

I think build

https://github.com/weavejester/integrant/blob/ec7730859e5f07f4849b72feb710e950c7c15232/src/integrant/core.cljc#L303

may solve this exact issue

(defmethod prep-foo :default [k config] config)

(defmethod prep-foo :database/postgresql [k config]
  (let [defaults {:my.database/primary {:port 4336}
                     :my.database/backup {:port 4337}}
  (merge defaults config))

(defn foo
  [config keys]
  (ig/build config keys prep-foo))
codestiff commented 4 years ago

I'm going to close this and experiment if this works.

codestiff commented 4 years ago

Thank you!

weavejester commented 4 years ago

No problem.

codestiff commented 4 years ago

Yeah, I think having separate build steps might be "overkill". It looks like having something like aero pass a config to integrant works well. Thanks again