-
Notifications
You must be signed in to change notification settings - Fork 64
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
pre-init-spec
before any initialization at all
#32
Comments
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 The way I've been considering solving it is to have a |
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
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 My custom solution was to validate myself everything after If you agree I can help you 👍 |
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. |
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 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 |
Originally I did call it 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 |
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 |
Yes, that's exactly what I was trying to refer to. Use Maybe it'll be clearer if I explain my ideal workflow :
In doing so I believe we are clearly separating concerns and it works for things in general. Maybe I am missing out something ? |
I made 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 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. |
If I understand correctly :
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. |
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.
Un-namespaced top-level keys aren't supported by Integrant anyway, so we're good on that front :) |
Well, then I like this solution ! I must admit I'm just not entirely sure what 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 :) |
Keeping the system as a map for development purposes might be a good idea. The If |
I might have some time to work on this issue soon, do you need some help ? |
If you like 😃 . Writing |
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.
The text was updated successfully, but these errors were encountered: