Support auto interval variables #71
Conversation
|
|
lint/variables.go
Outdated
| // If it is a template variable and we have a value, we use it | ||
| for _, v := range variables { | ||
| if v.Name != name { | ||
| continue | ||
| } | ||
| // if it has a current value, use it | ||
| if v.Current.Value != "" { | ||
| return v.Current.Value, nil | ||
| return expandVariables(v.Current.Value, variables) |
There was a problem hiding this comment.
I had to clone your repo and run through some debugging to wrap my head around this. I was worried about infinite recursion, but it'll just walk the tree until it either finds the templated value, or exhausts it's options!
This should also actually fix another issue I discovered where you have a multi-select value with the var $__all that didn't get expanded.
There was a problem hiding this comment.
Thanks a lot for the comment. Actually, there could be a case, if the template variable self references itself.
I don't know how grafana would handle it, but the linter was failing with a stack overflow. I submitted an updated PR for that.
There was a problem hiding this comment.
Ahh yes! That is a potential case. The solution is elegant. :)
Could I ask you to add a test for this? Where a template variable references itself, and the expected outcome is failure/empty.
Once we have that, I'll be happy to merge. Thank you for the contribution!
There was a problem hiding this comment.
I added a test, already :). I used it to trigger the stack overflow so I am sure it validates the behavior...
https://github.com/grafana/dashboard-linter/pull/71/files#diff-34ec875ff928ca836dfa602bba4346279462424c106b50f59016b04462883051R165
There was a problem hiding this comment.
Sheesh, you're right, I missed it, or github didn't render it.
That's what I get for reviewing before coffee. Thank you!
wuzuf
force-pushed
the
feature/auto-sampling-support
branch
from
eceeb3f
to
1f975c7
Compare
Hello,
One of my colleague reported another interesting case... When using "auto" option for interval variables, the variable definition is not a valid value but $__auto_interval_xxxx.
So I added support for this by:
__auto_intervalvariable with a 10s magic numberLet me know if you have questions/remarks,
Gabriel