Make types exportable (Bunch of changes) #3
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Due to the size of the change, I don't expect these changes to be merged upstream. But a number of them seem essential for actually using the package, so who knows. (1) Export some types In order to support dashboard declaration using the struct literal syntax, and just to set some properties at all, I exported a number of types: Board: - Templating - Annotation - Time - Timepicker - TemplateVar - Option - Current Panel: - SeriesOverride - Tooltip - Axis (2) Add omitempty tag to a number of non-required settings (prometheus) (3) Add the Target member to support Graphite (4) Use io.Reader as the parameter type to doRequest. Without that, it panics for GETs otherwise due to a nil-but-not-nil interface. (5) Concatenate the query string on the path rather than replacing it. This supports base URLs other than a bare hostname (6) Remove board member from Row type. The circular reference made it impossible to declare Boards and Rows using a struct literal.
|
FYI, this is the kind of use case I had for declaring and then uploading it var board = &sdk.Board{
Title: "App Stats",
Tags: []string{"app"},
Templating: sdk.Templating{
List: []sdk.TemplateVar{
{Name: "rollup", Type: "interval", Query: "1d,7d,1h", AllFormat: "glob"},
},
},
Time: sdk.Time{From: "now-7d", To: "now"},
Editable: true,
Rows: []*sdk.Row{
row("Performance / Availability",
span(6, graph("500s, measured by HAProxy",
"aliasByNode(summarize(infra.haproxy.backends.{a,b}.http5xx.count, '$rollup'), 4)")),
top10List("Top 10 500s", "http.appname.*.*.*.5xx.count", "2, 3, 4"),
),
row("Performance - Latency Histograms",
latencyHistogram("app", "action"),
latencyHistogram("app", "action2"),
),
},
} |
|
@robfig thank you! I'll take a look at the code soon. |
|
The proposed way for declaring dashboards looks reasonable. And on a first look it breaks nothing in old API. So I accept the changes they really improve the code. Thank you @robfig! "ExampleNewBoard" test is broken but it is fragile test I'll fix it later. |
grafov
added a commit
that referenced
this pull request
Mar 29, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Feel free to close if you don't want to merge, but I think these changes make this package a lot more usable for everyone. See the commit message for a summary of the changes
Thanks