Open markokocic opened 2 years ago
It looks like one of the target locations is expected to be a map, but it's not. The :merge
action expects a map at the target location.
I am trying to merge only to existing map entries, not create new ones. Here's the file, for the reference:
{:default
{:require-restart? true
:actions
{:assets []
:injections [{:type :edn
:path "deps.edn"
:target [:deps]
:action :merge
:value {io.github.kit-clj/kit-metrics {:mvn/version "1.0.0"}}}
{:type :edn
:path "resources/system.edn"
:target []
:action :merge
:value {:metrics/prometheus {}}}
{:type :edn
:path "resources/system.edn"
:target [:handler/ring]
:action :merge
:value {:metrics #ig/ref :metrics/prometheus}}
{:type :edn
:path "resources/system.edn"
:target [:reitit.routes/api]
:action :merge
:value {:metrics #ig/ref :metrics/prometheus}}
{:type :clj
:path "src/clj/<<sanitized>>/web/middleware/core.clj"
:action :append-requires
:value ["[iapetos.collector.ring :as prometheus-ring]"]}]}}}
The issue seems to be because I try to inject into system.edn multiple times. If I comment out multiple injections to the same file and leave only one, installation works.
Injecting multiple times into the same file should be fine. All the injections get aggregated here, and then the file is read once and transformed into a zipper. Then, the injections are injected into it and the file is written back out at the end. It's possible there's a bug in the logic though. :)
Ok, will check. Maybe it works and I messed up the config.edn, but I fail to see where is the error.
Btw, few more questsions:
modules-sync
just to try out the change.kit-generator
project has some tests defined, but no alias for running tests?It's definitely possible there's a bug in the code. I checked the other modules, and none of them do double injection for EDN files. And modules get cloned into the project, so the easiest way to play with them is to modify the files in the modules
folder in the project.
And I've been running tests via the REPL, but tests are work in progress at the moment. :)
And it does look like there's a bug here. I tried this and the second merge is outside the map.
(let [data (z/of-string "{:z :r :deps {:foo :bar}}")
updated (inject
{:type :edn
:data data
:target []
:action :merge
:value (io/str->edn "{:db.sql/connection #profile\n {:prod {:jdbc-url #env JDBC_URL}}}")})]
(z/root-string
(inject
{:type :edn
:data updated
:target []
:action :merge
:value (io/str->edn "{:x :y}")})))
It looks like topmost
function might be going a bit too far in case of EDN.
I added a fix that should let us test for now. I'd like to find a better way to do it in the future, but I think it should address the immediate issue. Let me know if it works.
Hi @yogthos , your fix seems to work. I was able to create a new module for metrics library.
It basically updates the project in the same way as by using +metrics
profile, with the exception that it doesn't update wrap-base
function in <<ns-name>>.web.middleware.core
package.
I guess injection framework can't manipulate functions yet. Not sure how to handle that. Maybe adding a new :note
key to module config that will contain a string explaining manual steps to be displayed after installation?
Good to hear the fix works, and function manipulation is something we could add relatively easily. There's already a version of that here for appending a build task calls. So, it could be generalized to allow specifying a namespace and function where the injection will happen.
If there were possibility to instrument functions or variables in Clojure code, then modules could do everything that profiles are used for now, rendering them obsolete. Then it would be possible to automatically install modules right after project creation.
It might be possible to use metadata for this, but would need a bit of research. It's also important to keep in mind that we have to handle assets other than code, such as HTML templates, configuration files, etc. I think profiles make sense for things that need to be decided at project creation time, but it's likely possible to express that using modules as well. So, the template could provide the minimal kernel of functionality, and then modules could be applied when the project is created based on the flags supplied. I'm generally partial to this idea as it removed the need for duplication between modules and profiles making maintenance easier.
Hi @yogthos, one compromise on this topic would be to keep generator logic / templating in profiles only to the parts that are not (yet) supported by modules, and move everything else to module. At the project generation time, generator can process e.g., HTML templates, or source code template, and after that invoke module installation of the corresponding module that will update system.edn
, add assets, requires, etc …
That would avoid duplication and make profile – module relationship 1 to 1.
The only drawback would be that if some functionality is not added immediately as a profile, it can't be added later through module (HTML template change, code change). Then it would need to point the user to the module doc page after installing the module to perform the manual step.
I was looking into this approach, but I'm not sure how to implement it in one step. Project generation creates a project in a subfolder, and one needs to cd
to that project to sync and install modules. There could be basically two approaches to touch this:
What do you think would be a better approach?
I tried to wrap kit/metrics library and wrap it into a new module. I have an issue when trying to install a new module using
kit/install-module
with the following error:When I comment out the lines containing non-empty target, install seems to work, but it's obviously not a solution.
I defined my module as following: https://github.com/markokocic/modules/blob/kit-metrics/metrics/config.edn
EDIT: The error message is also not very descriptive.