tools: add Makefile helpers for agent-flow-mixin #4338
Conversation
Signed-off-by: Paschalis Tsilias <[email protected]>
… compiled mixin Signed-off-by: Paschalis Tsilias <[email protected]>
| @@ -0,0 +1,5 @@ | |||
| { | |||
There was a problem hiding this comment.
Gets created when calling jb install but we never checked it in. As far as I can tell, other projects do check it in their repos, so I thought I'd do the same.
| @cd $(MIXIN_PATH) && \ | ||
| jb install && \ |
There was a problem hiding this comment.
I think you get jb install already from check-mixin-jb - if I understand this right.
| jb install && \ | ||
| mixtool lint mixin.libsonnet | ||
|
|
||
| check-mixin-jb: |
There was a problem hiding this comment.
Nit: I'd call it mixin-jb-install or sth like this,
| @cd $(MIXIN_PATH) && \ | ||
| jb install | ||
|
|
||
| check-mixin-mixtool: check-mixin-jb |
There was a problem hiding this comment.
Nit: This could be called lint-mixin 🤔
| PARENT_MAKEFILE := $(firstword $(MAKEFILE_LIST)) | ||
|
|
||
| MIXIN_PATH ?= operations/agent-flow-mixin | ||
| MIXIN_OUT_PATH ?= operations/agent-flow-mixin-compiled |
There was a problem hiding this comment.
What is the intended use case for the compiled mixin? If we're planning on including it as a release asset (which IMO we should), then I think this should go to dist/ instead of another file in the operations directory.
There was a problem hiding this comment.
My initial use case was to mention the make command in the clustering docs PR so that people could easily reuse our dashboards and alerts. TBH I hadn't thought about including this as a release asset.
So you think this should be invoked among the other commands in make dist? I think it makes sense; and once we've fixed linting errors we can also make it part of our Drone setup, much like other build artifacts.
There was a problem hiding this comment.
So you think this should be invoked among the other commands in make dist? I think it makes sense; and once we've fixed linting errors we can also make it part of our Drone setup, much like other build artifacts.
Hm, maybe? At first I thought that was what Mimir was doing, but I see they have the same -compiled folder as you do here.
I'll leave it up to you, having it as a release asset potentially makes the workflow of installing it easier.
|
@tpaschalis Can you take another look through your PR here and make any final touches? It looks like there's only a few comments from people and then this is ready to be merged. (Or close it if we don't need this right now and you don't have time to finish it) |
|
I'm closing this PR for now as I don't have the time to tidy this up. This is small enough for someone to get started with our build tooling, otherwise I'll revisit once there's a need for this. |
github-actions
bot
added
the
frozen-due-to-age
PR Description
As part of writing documentation for clustering, I'd like to simplify the way users interact with our agent-flow-mixin.
This is a first approach in doing so; allowing people to compile our mixin and have its output ready to be imported into their Grafana + Prometheus instances.
Which issue(s) this PR fixes
No issue filed
Notes to the Reviewer
In the future we should automate running
make check-mixininto our Github Actions workflow.Right now,
mixtool lintreports various issues, which can be fixed in another PR.PR Checklist