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

Discussion: replace start-* functions with single start function with options #19

Closed
aroemers opened this Issue Nov 26, 2015 · 8 comments

Comments

Projects
None yet
2 participants
@aroemers
Copy link

aroemers commented Nov 26, 2015

First of all, mount seems a great idea. I am currently refactoring a stuartsierra/component app to mount's defstates, in order to evaluate it. I like how stuff becomes more clojuresque. What I am missing though are two things (also mentioned in the following issues: #2 #8 #9)

  1. A solid way to pass arguments to defstate's start functions. I am not so fond of the with-args approach, binding them to a single var/atom in the mount.core namespace.
  2. A single data source that defines how the modules compose, and which states are started. My use case is having a "standard library" of module namespaces, and reusing those in actual apps. In mount's current state, some of this can be achieved, by using functions like start-with, start-without, but those do not compose. And even if they did, the required nesting/composition is not really programmatically configurable.

So, I was thinking, what about a single start function, taking a map for mix-n-matching the available options? This also makes mount more extendible with new features, without the need to update or add new API functions.

For example, what about this?

(mount/start
 {;; Only start the following state vars, replacing `(start <state>+)`
  :only [#'my.mount.config/config]
  ;; Or, start all known state vars, except the following,
  ;; replacing `(start-without ...)`
  :except [#'my.mount.db.datomic/connection]

  ;; Substitute some states' start and stop functions with those of
  ;; other states, replacing `(start-with {...})`
  :substitute-with-var   {#'my.mount.config/config #'my.mount.config.map/data}
  ;; and/or substitute some states with fixed values (new feature)
  :substitute-with-val   {#'my.mount.config/config {:my 'config-data}}
  ;; and/or substitute some states' start and stop functions with
  ;; those specified here directly (new feature)
  :substitute-with-state {#'my.mount.config/config {:start {:my 'config-data}}}

  ;; Specify arguments for state start functions, expecting those
  ;; functions to have (at least) a 1-arity version.
  :arguments {#'my.mount.config/config         {:resource "/config.edn"}
              #'my.mount.db.datomic/connection {:config-var #'my.mount.config/config}
              #'my.mount.config.map/data       {:my 'config-data}}})

This composes all options nicely, and the data structure can be passed around, read from a file and/or altered with Clojure's core functions (as advocated in https://www.youtube.com/watch?v=3oQTSP4FngY). I think this approach opens up the possibility of composing the modules better, enabling this "standard library" approach.

Of course, if you still want to offer a more function-based API, one could add "builder" functions, such as:

(-> (mount/with {#'my.mount.config/config #'my.mount.config.map/data})
    (mount/without #'my.mount.db.datomic/connection)
    (mount/with-args {#'my.mount.config/config {:resource "/config.edn"}})
    (mount/start))

I am very interested in your opinion, and if you like it, I'll try to find the time to write the actual code for this idea.

@tolitius

This comment has been minimized.

Copy link
Owner

tolitius commented Nov 27, 2015

@aroemers: great feedback! thank you.

The reason there are multiple start APIs (start/start-with/start-without..), is because I wanted to keep (start) that can start all and arbitrary number of states. Similar to Clojure functions that can work with & args. So when #2 idea came around, I did not want to restrict (start) to start all, and have (start args) to start with args. My thinking was that there are more things we'll want from (start*), and overloading it with multiple meanings will lose simplicity and the clear intent of what it does. (start) and (start state1 state2 ...) seem to reflect the expectations just by looking at them.

Having said that, I did think of something similar as your first example:

(start :args args
       :with {from to}
       :without #{...}
       :only #{...})

But I believe there are some disadvantages to it:

  • it asks for more DSL (from your example: resource / my / config-var, ..)
  • nesting needs to be right (i.e. substitute-with-state from your example)
  • it does not feel composable, it is more of a "configurable" or "mergeable" solution

"A single data source that defines how the modules compose, and which states are started."

I'd like mount to be as invisible to the app as possible. I believe it is up to a developer (application) to define how the modules compose, and by using :require in the app to express which states are needed (to start). mount's main job would be to come in and start them. This way app does not know anything about mount: everything besides defstate remains to be "just Clojure".


With all the above, you bring a very good point, which I did not accomplish yet: all the ways to start states should compose. While working with mount for a month ( that's how old it is :), I did not really need them to be composable, but I can clearly see how much more flexibility it would have with having start* API be composable.

I do like your builder idea, we can have with/without/with-args/with-xyz to take args and a set of states and return a modified set of states:

(defn with [subs & states] ..) ;; returns states
(defn without [excludes & states] ..) ;; returns states
(defn with-args [args & states] ..) ;; returns states

the we can do something like

(->> (with {#'app.nyse/db  #'app.test/test-db
            #'app.nyse/publisher #'app.test/test-publisher}) 
     (without #{#'app/nrepl #'app.feeds/feed-listener})
     (with-args args)
     start)

I also like

  • substitute some states with fixed values (new feature)
  • substitute some states' start and stop functions with those specified directly (new feature)

these can be added to the start-with and the new with. They would need more design thinking in terms of internal implementation to coexist with start-with states.

@aroemers

This comment has been minimized.

Copy link

aroemers commented Nov 27, 2015

@tolitius Good to hear you appreciate my feedback!

The reason there are multiple start APIs (start/start-with/start-without..), is because I wanted to keep (start) that can start all and arbitrary number of states. Similar to Clojure functions that can work with & args. [...]

That makes sense. I fully respect that, and it makes starting and stopping states in the REPL more workable as well.

[..] But I believe there are some disadvantages to it:

  • it asks for more DSL (from your example: resource / my / config-var, ..)

I am not sure what you mean here. What DSL do you mean? As far as I can tell, there is no DSL in my example, just a bunch of specified and documented keys, filled with example data. Mount currently and in my example does not and should not care for example about what data is passed in the :arguments/with-args option.

  • nesting needs to be right (i.e. substitute-with-state from your example)

Again, I am not sure I follow. One of my main motivations for suggesting to use a data structure, is exactly because it does not need any nesting and becomes declarative. The order in which the options are "processed" is a hidden implementation detail.

  • it does not feel composable, it is more of a "configurable" or "mergeable" solution

True, the word composable is not very suitable in this sense.

I'd like mount to be as invisible to the app as possible.

I understand. Most of the time, just calling (mount/start) should be enough.

I believe it is up to a developer (application) to define how the modules compose, and by using :require in the app to express which states are needed (to start)...

And this is exactly where my "problem" lies. Just :require-ing the namespaces with the states you'd want to start is not enough for defining how modules compose. Say for example that I have datomic namespace, which reads from a config namespace what URI to use for the connection. That config namespace reads from the environment for example. Now let's say I want to reuse this datomic namespace in another project, but use another (compatible) namespace for configuration, e.g. file-config. Now I have to change the actual source code of the datomic namespace in order to do this, or use start-with. I'd rather externalize this in a configuration.

... mount's main job would be to come in and start them. This way app does not know anything about mount: everything besides defstate remains to be "just Clojure".

True, which is what I like about mount. Does my proposal change this? Maybe, indeed, my approach leads to the Inversion of Control/Dependency Injection "framework" you are trying to avoid. If so, your suggestion of using my proposed "builder" pattern may indeed be the way to go. Still, having a single data structure (internally) might still prove useful, as I'll explain next.

I do like your builder idea, we can have with/without/with-args/with-xyz to take args and a set of states and return a modified set of states
...
I also like substitute some states with fixed values (new feature) and substitute some states' start and stop functions with those specified directly (new feature) these can be added to the start-with and the new with. They would need more design thinking in terms of internal implementation to coexist with start-with states.

Good to hear you like it. You propose the "builder" functions to return modified states. Maybe I did not fully understand or misread your example, but doesn't this make (some) of them side-effectful? Such as mount/with? If so, this makes them not really "builder" functions. Having a data structure being build up, does.

Oh, and I would rename start-without to start-except 😄 In my opinion this is more in line with stop-except and does not look as if it is the counterpart of start-with.

@aroemers

This comment has been minimized.

Copy link

aroemers commented Nov 27, 2015

@tolitius The more I think about it (and tinkering with above ideas here), the more I get the impression that mount is simply not suitable for libraries, and does not belong there. It really is for apps. This is different from component, hence me starting this discussion.

So, maybe you'll still find some useful points in this discussion, but my main issue may be a non-issue after all 😄

@tolitius

This comment has been minimized.

Copy link
Owner

tolitius commented Nov 27, 2015

@aroemers: I will respond with details later to continue this discussion, it's really good :)

meanwhile two things:

  • I like the idea of passing args and subs along with the final set of states to start
  • I wanted to understand why you don't think mount is suitable for libraries. Can you explain?

tolitius added a commit that referenced this issue Nov 29, 2015

@aroemers

This comment has been minimized.

Copy link

aroemers commented Nov 30, 2015

@tolitius Looking forward to your response :)

I wanted to understand why you don't think mount is suitable for libraries. Can you explain?

Let's again say I have two namespaces: datomic and config. When I would use defstate in those generic and reusable library namespaces, it could look like the following:

(ns config)

(defstate config
  :start (clojure.edn/read-string (slurp (clojure.java.io/resource "/config.edn"))))
(ns datomic
  (:require config))

(defstate connection
  :start (datomic.api/connect (get config :datomic-uri)))

(defn write-schema [schema]
  (datomic.api/transact connection schema))
(ns app
  (:require 'datomic))

(mount/start)
(datomic/write-schema ...)

Works like a charm and looks very clean. But, now the datomic/connection state is bound to always use the config namespace. To overcome this, one needs to use composable functions (which mount already has, but more from the standpoint of testing) and/or configuration maps to manage how states use each other. In other words, dependency injection.

The same example as above, can also be rewritten with leaving out the defstates from the library namespaces, and move them to the app. This might look something like this:

(ns config)

(defn load-file []
  (clojure.edn/read-string (slurp (clojure.java.io/resource "/config.edn"))))
(ns datomic)

(defn make-connection [uri]
  (datomic.api/connect uri))

(defn write-schema [connection schema]
  (datomic.api/transact connection schema))
(ns app ;; one or many of course
  (:require 'datomic 'config))

(defstate config
  :start (config/load-file))

(defstate datomic-connection
  :start (datomic/make-connection (get config :datomic-uri)))

(mount/start)
(datomic/write-schema datomic-connection ...)

In above example, the library namespaces don't need any mechanism anymore to "inject" the correct states or namespaces anymore. It is left to the app, by plain defstates, without any configuration. That is what I meant when I said that library namespaces are not suitable for mount's defstate.

Agree? Disagree? Let me know :)

@tolitius

This comment has been minimized.

Copy link
Owner

tolitius commented Nov 30, 2015

@aroemers: I think one of the things that makes a library good is to either have no state or to have some internal state that does not leak out.

In your example, if I understand what you meant correctly, you'd like to keep the creation of datomic connection within a library, which makes sense.

The way I would do it though is not via defstate, but just via a function:

(defn connect-to-db [conf]
  (datomic.api/connect (get-in conf [:datomic :uri])))

and in the app itself I would use this function from a library to create a state (i.e. connection):

(defstate db-conn :start (connect-to-db conf) 
                  :stop (close db-conn))

whether I include a conf state in the same namespace as db-conn would be a matter of an application design. Most likely this configuration state will be used by many other states beyond db-conn, so it might make sense to keep it elsewhere.

If you meant that defstates are not for libraries in a sense that "libraries should not keep exposed state", I agree with you. It would not be so much about mount though, but it would rather be a design decision not to have any state in the library that is exposed to the client.

@aroemers

This comment has been minimized.

Copy link

aroemers commented Dec 1, 2015

@tolitius Yes, that is exactly what I meant. So I think we agree here, and my initial misconception or urge to have more configurability comes from the way component influences application and library design.

Well, maybe something good came out of this discussion anyway (such as the :replace-with-val and :replace-with-state options), and having the various start-* functions compose (e.g. builders?) may still be a good idea. I am curious to see if and how these ideas may end up in mount. We can close this issue I think?

@tolitius

This comment has been minimized.

Copy link
Owner

tolitius commented Dec 1, 2015

@aroemers definitely, great discussion!

feel free to open a new issue about the composability of start flavors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment