`pre-init-spec` before any initialization at all

[helins]

In the current workflow, integrant initializes keys one by one, checking each time if the provided data is valid according to pre-init-spec. When the data is not valid, integrant throws and it actually gets messy. For instance, the resources allocated by the previous keys are not released. Sometimes, this means we have to restart the repl.

What I suggest is that integrant validates all data before initializing anything. The specs we write for our system means that if the data is not valid, something will probably fail. It is best to fail fast instead of starting the system, opening ports and stuff', and eventually failing anyway and leaving a big mess.

[weavejester]

That functionality is planned, but unfortunately very difficult to implement.

Consider a configuration like:

{:duct.database/sql   {:connection-uri "jdbc:sqlite:db/dev.sqlite"}
 :app.handler/example {:db #ig/ref :duct.database/sql}}

If we try to validate the spec for :app.handler/example, then we need to know that the :duct.database/sql key produces a database spec.

The way I've been considering solving it is to have a post-init-spec, and use generators associated with the specs produced to replace the refs. However, not all objects are easy to generate without side effects, such as database connections, for example.

[helins]

Thanks for quickly answering. I should probably read the source code but I think what I'm about to say makes sense.

Let us make a distinction between the data specific to a key and its dependencies :

{:duct.database/sql {:connection-uri "jdbc:sqlite:db/dev.sqlite"}
 :app.handler/example {:db #ig/ref :duct.database/sql
                                     :something 42}

We can say :something is data specific to the key :app.handler/example whereas :db is a dependency, a reference to another key, and many other keys can rely on it.

Now, I (and I guess other users) need spec for

• Validating specific data which is just EDN.

• Making sure I haven't forgot to declare a needed dependency, ie. validating the presence of the needed integrant references, and not what those dependencies have produced during their own initialization.

  If we wish to validate the result of :db, what will actually be passed to the initialization of :app.handler/example, this should actually be done using something akin to post-ini-spec after the initialization of :duct.database/sql itself and not while initializing its dependent keys. Plus, we don't want every dependent key to validate the result of :duct.database/sql, it should be done only once anyway.

My custom solution was to validate myself everything after ig/read-string and before ig/init. I've even written a short macro for spec'ing integrant references and the rest is just spec'ing specific data. This is simple, so why am I here ? Well because as you say in the readme, we don't want to write specs directly against the keys and ig/init has a second arg for selecting the keys we want to use, meaning the spec for our config map should be indeed dynamically generated. pre-init-spec offers a simple way for doing so and avoiding confusing boilerplate.

If you agree I can help you :+1:

[weavejester]

I'd rather not add functionality that works for some things and doesn't work for others. Ideally a feature should have consistency.

I'll give this some thought.

[helins]

Well, I might have a hard time understanding exactly why you would want a child to check if its parent is behaving as expected, I would say it's the responsability of the parent to respect its post-init-spec contract. For my uses cases it's more about ensuring that the dependency is declared in the config file, the kind of file we modify very often.

I guess what I'm really talking about is something to ensure the config file makes sense before the system even start, almost a compile time thing. Ensuring that the initialization is going well (what you want ?) is actually another topic and I would rather call that init-spec.

[weavejester]

Originally I did call it init-spec, but I then realized that a post-init-spec method might be necessary in future. The pre-init-spec method is called just before the map entry is initiated, hence the name.

I agree that a spec that checks the configuration in a side-effectful way would be useful. I'd like to consider how to do this in a way that is consistent and predictable. If you factor out part of the configuration into a reference, ideally the same spec should work.

You could write specs directly against the keys if you wished. It might not be semantically accurate, but then you could validate the configuration with (s/keys).

[weavejester]

An alternative would be to have a way of validating the spec before it's been touched in any way, and incorporate the idea of validating refs themselves. For instance, you wouldn't say :db should be a database connection; you'd say :db should be a ref to :duct.database/sql or something that inherits from it.

[helins]

Yes, that's exactly what I was trying to refer to. Use ig/read-string and validate the whole thing yourself before ig/init, where specs for dependencies such as :db would indeed be validating refs, "integrant references" as I say. Ideally, I propose this should be part of the library, and it should happen when calling ig/init.

Maybe it'll be clearer if I explain my ideal workflow :

• The user calls ig/init with or without a list of keys (in dev I often work with only a few keys at a time).

• Using spec, validate all the selected keys to check if all of it makes sense (each keys contains the refs it needs + its own data). That's what I would call pre-init-spec, but I understand you reserved a different meaning for that. Whatever the name, it happens indeed before proper initialization.

• If everything is valid, the configuration makes sense, let us init the system using init-key. We can be quite confident it will probably behave as expected, go smoothly, because the configuration is alright.

• Every time a key is initialized, call post-init-spec on it. Often it is a good idea to ensure a parent respect a contract, so that we clearly define what the children will receive. If a parent doesn't respect it's contract, okay, let's throw. It will halt the initialization and leave everything in a mess, but that's the point. Fail fast.

In doing so I believe we are clearly separating concerns and it works for things in general. Maybe I am missing out something ?

[weavejester]

I made pre-init-spec a multimethod so that it could be inserted into the act of initiating the configuration, but if we want to validate the configuration before it's been initiated, I don't think we need to use multimethods at all.

Instead, consider a syntax like:

(require '[clojure.spec.alpha :as s]
         '[integrant.core :as ig]
         '[integrant.spec :as is])

(s/def :adapter/port    pos-int?)
(s/def :adapter/handler (is/ref :adapter/handler))
(s/def :adapter/jetty   (s/keys :req-un [:adapter/port :adapter/handler]))

(def config
  {:adapter/jetty
   {:port 3000, :handler (ig/ref :app.handler/example)}
   [:adapter/handler :app.handler/example]
   {}})

(s/valid? (is/derived-keys) config)

We introduce a integrant.spec namespace, and two new specs: is/ref and is/derived-keys. The former specifies a reference derived from the keys specified as its arguments, and the latter specifies a map of derived keys (like s/keys but accounting for derived keys and composite keys).

Once we have a spec we can validate as normal. This does mean that the running system won't adhere to the spec, but on the other hand the running system is considered to be opaque, so we wouldn't be checking it anyway.

[helins]

If I understand correctly :

• is/ref is for spec'ing references, easy peasy.

• is/derived-keys produces a spec validating a config map according to the keys it contains, while taking care of derived keys. It's quite dynamic without needing multimethods "à la pre-init-spec". However, two things to ponder. First, is it okay to spec against keys, given those keys means different things at different times ? Second, what if someone wants un-namespaced keys (I personally don't) ?

Anyway, certainly a step in the right direction. I don't mind the running system not adhering to those spec, I don't see why it always should.

[weavejester]

First, is it okay to spec against keys, given those keys means different things at different times ?

Yes, it does mean that a running system won't conform to the spec. However, a running system is designed to be opaque, in that you can only halt it. The fact that it's also map is an implementation detail. Given this, I now think that it's fine to spec directly against the keys, and in future perhaps I'll add a wrapper around the running system to enforce it's opacity.

Second, what if someone wants un-namespaced keys (I personally don't) ?

Un-namespaced top-level keys aren't supported by Integrant anyway, so we're good on that front :)

[helins]

Well, then I like this solution !

I must admit I'm just not entirely sure what pre-init-spec would be really for. For instance, the example given is the readme would be more relevant using this integrant.spec namespace.

As to the fact that a running system is a map, I find it actually quite useful. You might call it a leaky abstraction, but it actually helps very much during dev. I guess it's a discussion for another time :)

[weavejester]

Keeping the system as a map for development purposes might be a good idea.

The pre-init-spec is useful for validating the configuration value after references are resolved. For example, you could make sure that the :db key really is a database connection.

If init fails, ideally we should clean up the mess afterwards, halting the partially started system. That's currently on my todo list for Integrant-REPL.

[helins]

I might have some time to work on this issue soon, do you need some help ?

[weavejester]

If you like 😃 . Writing is/derived-keys might be somewhat tricky, though.