-
Notifications
You must be signed in to change notification settings - Fork 21
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
Can't put module config in a profile #20
Comments
I'll add that |
Unfortunately this currently isn't possible without adding a fourth layer to how the system is processed. Currently Duct turns a module configuration into a component configuration into a system:
At the moment profiles are implemented as a type of module. Separating modules and profiles would require a fourth layer, e.g.
That's almost how 0.10 worked, except the profiles, modules and component layers were all mixed together in a haphazard way which caused a number of issues when the different pieces interacted. Could you give a little more information as to what you're trying to do? |
OK, understood! So for configuration that differs between profiles, should I just be changing top-level keys instead of module options, like below? config.edn
foo.clj:
I was essentially doing the following instead, using module options rather than top-level keys: foo.clj (previously):
|
Right. To me that seems the simpler solution, since it doesn't mean separating profiles from modules. However, if it turns out to be particularly clunky, I'd be willing to think about other solutions. I'd be very interested in some real-world examples, since it's hard to get a good feeling of what is most suitable when there's only small "foobar" code snippets. |
Got it, thank you! My use case really is not any more sophisticated than the example I provided, but I sadly can't share any real code until I can convince my employer to open-source the project (some day maybe). Right now it's not really any better or worse to do it the suggested way, but it might be helpful to have some kind of disclaimer about not being able to configure modules via profiles in your documentation. (Unless you have that somewhere and I missed it?) I did see that your web module has some kind of reference to the "environment" but I couldn't follow the logic... it's hard enough for me to keep the other two distinct notions of "profiles" straight in my head (leiningen profiles vs duct profiles). |
Ah actually I lied, this does turn out to be slightly inconvenient, because my :foo/bar key has some simple logic attached to it (like a default value, or casting the value, or some such). Of course, you could do this with a
So yeah I could just use init-keys for |
Have you taken a look at |
I had, and I just reviewed the information I could find on it. It sounds like, as you said, modules can sometimes be replaced entirely by profiles+prep-key? But I'm unclear on some things: So, consider my module (:duct.module/foo) that adds some keys (my :foo/bar and :foo/baz) above. You could have a profile that adds those keys too (call it :duct.profile/foo). But say I want them to be added by including :duct.profile/foo and then customized in other profiles (:duct.profile/dev vs :duct.profile/prod). To my knowledge, there's no notion of profiles overriding each other in a hierarchy, right? So dev and prod can't just override foo predictably, can they? Assuming that's not the approach you intended for profiles, that leaves me using a module (:duct.module/foo). It could add the keys as above. But to get the default-value (or similar) behavior above, I'd have to use prep-key, because I want dev and prod to be able to override the values (and as you said, I can't customize a module qua the module in profiles, I'd have to use top-level keys.) So I'm left with the following, I think: config.edn:
foo.clj:
Is that correct? It feels awkward and verbose. Is there a better way? |
It's really hard to suggest a solution without knowing more about the problem. Is there any way you can talk about your use case in slightly more specific terms, without revealing any code? Using "foo" keys tells me roughly what you're trying to do, but not why. |
I really have been striving to convey the "why"... I'll try to boil it down to a list of needed features:
Does that help? |
This seems like it would be a totally normative use case concerning how modules are to be developed... again, your web module seems to do some sort of magic to reference the dev vs. prod environment beyond the above stated mechanisms but I can't grasp it yet. |
That helps a little, and I can at least give you a few options, though without further information I can't say which is the best solution for your problem. Option 1The first option is to do the same thing as Duct's middleware, which is to look for a key in the configuration called (defmethod ig/init-key :foo.module/example [_ _]
(fn [config]
(if (= :development (:duct.core/environment config))
;; add dev setup to config
;; add prod setup to config
))) Option 2If that's too coarse-grained, and you specifically need options in your config, then you can add a key that is just for passing per-profile options to the module: (defmethod ig/init-key :foo.module/example [_ top-level-options]
(fn [config]
(let [profile-options (:foo.module.profile-options/example config)
all-options (duct.core/merge-configs top-level-options profile-options)
...))) Option 3The third option is not to use (defn read-config []
(duct/merge-config
(duct/read-config (io/resource "todo/config.edn"))
(duct/read-config (io/resource "dev")))) Then remove these lines from your config: :duct.profile/dev #duct/include "dev"
:duct.profile/local #duct/include "local" And instead have a {:duct.profile/local #duct/include "local"
:duct.profile/dev
{...}
:foo.module/example {...}} |
Additionally, I'll give this problem a little more thought and try to come up with a more general solution. |
Thank you! Here are my questions: Option 1:This seemingly moves the dev/prod decision-making to the module itself, rather than the configuration. So the module developer can build in dev/prod differentiation, but the module user can't? Unless I included a module option like so: config.edn:
foo.clj:
Doable but pretty awkward, given that we already have a mechanism for differentiating dev vs prod config (the profiles). Option 2:As I consider it more, I think this is how you would address the concern above, right? Option 3:This seems like the most straightforward path, and I may end up going with it for now... If what I originally assumed was going to work could be made to work (modules configured in a profile), that would be better than any of these workarounds. It was hard enough for me to grok the concept of how modules and profiles and duct itself all work. Once I got there though, I had such a nice, tidy mental model of the system, because duct is really elegant on its surface once you get a wide enough perspective of it. But then I hit this issue and my mental model had to suddenly become more complex and nuanced. |
Is it more than just resolving all profiles first, and then taking that config and resolving all non-profile modules next? |
Re-reading your earlier responses, I see that's what you said would be the needed solution. Is that difficult or error-prone, or might it have unwanted side-effects? Or is it a real option? |
Let me see if I can explain the problem a little more. In previous versions, it was possible for a module to reference another key. For example: {:component/foo {:something 1}
:module/bar {:config #ig/ref :example/foo}} In order for the module To solve this, Duct 0.11 has a clear separation between modules and non-module keys. When you write a configuration in 0.11, all of the top level keys are modules. Behind the scenes, profiles are just modules that merge in their value into the configuration: (defmethod ig/init-key :duct/profile [_ profile]
#(duct/merge-configs % profile)) (It's a touch more complicated in practice, but not by much.) So I want to maintain hygiene between modules and normal keys. I don't want to get into a situation where modules and non-module keys (which I'll call components) are allowed to reference each other. One solution is to add in a new profiles layer, so instead of writing: {:duct.module/logging {}
:duct.profile/base {:duct.core/project-ns example}
:duct.profile/dev #duct/include "dev"} We might instead write: {:base {:duct.module/logging {}
:duct.module/merge {:duct.core/project-ns example}}
:dev #duct/include "dev"} But I don't think that looks particularly elegant. Another possible solution is to write modules like this: (defmethod ig/init-key :duct/module [k opts]
(fn [config]
(let [opts (merge opts (config k))]
...))) So a module looks for its own key in the configuration to get extra options. This means you could write: {:duct.profile/dev
{:example.module/foo {...}}
:example.module/foo {}} However, the unintuitive part is that the module needs to be at the top level as well as in the profile. I think this would be a big source of confusion. So in a nutshell: we ideally want to put module configuration in profiles, but we also don't want to mix module keys with component keys. That's what makes the problem rather difficult to approach. |
OK, I can definitely see the difficulty. You know what though, I totally forgot about I'm sure though that there are cases where this issue will cause a harder problem for the module developer, but so far I don't think I've actually run into any... At the very least, it's probably worth a mention in your documentation and flagged as a "gotcha." Like I said, it was intuitive for me to try it and surprising that it didn't work. |
Thanks for the update. I'm glad you found an alternative solution. I'll give this a little more thought and see if I can come up with something that feels more intuitive while still maintaining hygiene between modules and components. |
Thank you for being so attentive to my questions! I wish all open source maintainers were as on the ball as you are. |
I'm revisiting this issue because I'm now thinking about a use case where, depending on the profile, some modules may or may not be desired. It's totally possible to just have a completely different config.edn for each separate case, I suppose. But my first thought was "wouldn't it be nice if I could do this specialization via profiles" since, like I've said before, profiles are an "obvious" mechanism to do such things from an outside perspective. Separate config.edns would also either cause duplication problems or involve some increased complexity by needing a merging strategy. Another option I'm mulling over is to include all the modules anyway but, depending on an env variable, either add or don't add that module's keys. This also works, but it's a "code" solution whereas "data" solutions are of course preferred. (Although I already use a similar approach for having multiple "flavors" of each module, like "IP vs Serial" for a given communication protocol.) And speaking of which, I'm not sure why I haven't shared this yet, since it's just a sketch of what my application does, but here's my concrete use case: My application is an automated test system that targets embedded devices. My integrant system manages several stateful interfaces for communicating with these devices over various protocols, and I just use Midje to write the tests themselves that exercise these protocols. Up until now, I've only had one type of target device, which always needs certain interfaces (Modbus, BACnet, and an in-house UDP protocol, and each can have several "flavors" as above.) But now I'm targeting multiple different types of devices, which all require a different assortment of interfaces (and some new ones: CANopen and discrete/analog I/O.) The necessary bits for each protocol (BACnet, Modbus, that UDP thing, CAN, discrete/analog) are currently collected into a separate module for each. As mentioned, the currently-configured "flavor" of each profile and other device-specific config (like IP address, COM port, etc.) are determined by env variables. |
Whoops didn't actually include an actual question in there! The basic question is this: Is there a good, data-driven way to turn these various features/modules/interfaces on and off, other than having separate config.edns? |
One option is to have a key in a profile that the modules look for to determine whether or not they should be on or off. Profiles will be merged before modules (because modules implicitly depend on the set of profiles), so modules can check any key set by a profile. I'm going to think about this a little more, but it might take a while. |
I'm hoping we can come up with a nice, intuitive solution, but until then I'm getting by doing the kinds of workarounds we've been discussing. |
I got bitten by this again. My module needs to add different keys to the system based on config that changes per profile, and I don't think that's possible with duct right now. (Please correct me if I'm wrong.) I find myself wishing that profiles and modules were two different layers, with profiles happening first. Here's the general pattern: config.edn: {:duct.profile/base {
:duct.core/project-ns myproj}
:duct.profile/dev #duct/include "dev"
:duct.profile/local #duct/include "local"
:duct.profile/prod {}
:duct.module/logging {}
:myproj/module #ig/ref :myproj/config} local.edn: {:myproj/config [:a :b :c]
} myproj.clj: (defmethod ig/init-key ::module [_ mylist]
(fn [system]
(duct/merge-configs system
(reduce (fn [m k]
(assoc m k {:config-goes :here}))
{}
mylist)))) This yields |
Moving the |
This design is the most promising fix to the module/profile problem. I haven't had time to implement it, but that's what I was planning on going with. I'd like to get your opinion on whether that would work well for you, and what you think of the syntax. |
That would be a huge step forward. Ideally you wouldn't need the |
I have some module config that varies between :duct.profile/dev and :duct.profile/prod. But the module is not initialized when it exists only in those profiles.
config.edn:
Now if I forget about profiles and just put the module config directly in the top-level of config.edn, the module works as expected.
Is this supposed to work and am I probably missing something?
The text was updated successfully, but these errors were encountered: