duct-framework / core

The core library of the Duct framework
75 stars 21 forks source link

duct.core/prep initializes modules #6

Closed flyingmachine closed 5 years ago

flyingmachine commented 7 years ago

Don't know if this is an actual issue, but I found it surprising that duct.core/prep initializes modules. "Prep a configuration, ready to be initiated" to me implies that the config returned hasn't had ig/init called on any part of it (though I understand the rationale behind using init to apply a module).

In my app, my side-effecting datomic config ended up getting initiated. I have a module :sweet-tooth.endpoint/group that lets you define a bunch of handlers along with the option they should all receive:

:sweet-tooth.endpoint/group
{[:ca.endpoint/flag
  :ca.endpoint/vote
  :ca.endpoint/notification]
 {:db #ig/ref :sweet-tooth.endpoint/datomic}}

In this case, applying the module led to :sweet-tooth.endpoint/datomic getting initiated, and the system trying to connect to datomic.

weavejester commented 7 years ago

I've been considering this issue for a while. The way I see it, there are four options:

  1. Initiate and substitute in any refs associated with the modules (the current behavior).
  2. Substitute in the un-initiated value of refs.
  3. Introduce a new type of ref for un-initiated values.
  4. Don't do anything with refs, but perhaps introduce a function to manually resolve them.

I'm starting to become inclined toward the last option, as that functionality mirrors that of macros. If refs are equivalent to symbols, and initiating a ref is equivalent to evaluating a symbol, then in a macro they'd remain unevaluated unless eval was manually called.

Thoughts?

flyingmachine commented 7 years ago

I like 4 as well, and I like the macro analogy. What do you think of creating a new function, something like init-module-key that is similar to ig/init-key but only inits keys derived from :duct/module? That lines up even more with my intuition about how this would work; in the same way macros get expanded without having to manually eval them, modules would get expanded.

weavejester commented 7 years ago

We could just use init-key and call it directly instead of going through init. This kind of has a parallel with macros, which are effectively just functions with a macro flag set on them.

Alternatively, we create a prep-key multimethod or something similar.

flyingmachine commented 7 years ago

One issue I could see with overloading init-key is that it might be confusing because people learn about it in the context of initializing a system. The majority of the documentation presents it as providing the state transition from uninitialized to started (roughly, sorry if I'm getting the terms wrong), so learning that it can also provide the transition from unprepped to uninitialized might cause some head scratching. Then again, the macro metaphor might be good enough to explain what's happening.

I am leaning a little toward prep-key - duct introduces something new, and it seems easier to work with if it's kept distinct. Honestly, though, I think either approach would work fine :)

weavejester commented 7 years ago

I'm leaning toward prep-key also, though I'm currently undecided on just what it would look like.

WhittlesJr commented 6 years ago

Has there been any further thought on this issue?

weavejester commented 6 years ago

The current plan is to redesign how modules work. Rather than transforming the configuration through a chain of pure functions, I'm toying with a design that resembles Leiningen's profiles. The idea is to get broadly the same functionality through simpler tools.

Currently we have a :duct.core/include key to pull in external resources:

{:duct.core/include ["dev"]}

I don't like the idea of a key with special behavior. My intent is to replace this with a reader tag:

{:duct.profile/dev #duct/include "dev"}

This tag reads in the "dev" configuration and places it under the :duct.profile/dev key. It's resolved during the read stage.

The question is how do we best take what's in :duct.profile/dev and merge it into the parent configuration? This is the part I'm undecided upon.

However, assuming we have some merge strategy we're happy with, we can apply the same idea to modules. Instead of modules transforming the configuration, they produce a profile that's merged in.

This means we can define a prep-key multimethod very simply:

(defmulti prep-key
  (fn [k v] (ig/normalize-key k))

The prep stage simply replaces each value with the output from prep-key before it is initiated. It does not resolve references, but you can add references. This means we can do some interesting things like:

(defmethod duct/prep-key :foo.handler/things [_ v]
  (update v :db #(or % (ig/ref :duct.database/sql))))

So if the :db option does not exist, automatically add a reference to :duct.database/sql.

WhittlesJr commented 6 years ago

Fascinating... I'm really looking forward to that! Thank you for the update!

flyingmachine commented 6 years ago

What if you made :duct.core/include a module that takes configs loaded by #duct/include as an arg?

RickMoynihan commented 6 years ago

@weavejester What do you see the advantages are of moving include to the read stage? One disadvantage might be that it prevents an application from preventing includes from happening.

I also wonder whether it might be worth enforcing a naming convention on includes to e.g. namespace them or somehow prevent accidental recursive loops across modules.

Regardless of include, which I think will always be necessary, useful, and occaisionally problematic; I like the idea of profiles. I also think it might be worth considering making duct.profile keys work with either a hashmap on their right hand side, or a vector of profile keys. In the case of a vector the keys (which might want to be #ig/ref's) would be dereferenced and meta-merged in the order they were provided, as with lein profiles.

weavejester commented 6 years ago

What do you see the advantages are of moving include to the read stage?

There are a few:

  1. It separates reading the configuration from transforming it.
  2. It decomplects meta-merge from reading.
  3. It removes the need for side-effects when building the configuration.
  4. It allows the user to include smaller parts of a config, not just top-level keys.
  5. It removes the need to treat the :duct.core/include key specially.

With #duct/include and :duct/profile, we're effectively separating out the acts of reading an included configuration, and meta-merging it into the configuration.

One disadvantage might be that it prevents an application from preventing includes from happening.

While that's true, if the include loads into a profile you can just ignore/remove the profile. It might be slightly less efficient, since you're still reading the file, but in practice that shouldn't matter.

I also wonder whether it might be worth enforcing a naming convention on includes to e.g. namespace them or somehow prevent accidental recursive loops across modules.

We could just raise an error on a recursive loop.