Skip to content
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

feat: Description fetched from ontap docs via cli #2454

Merged
merged 7 commits into from
Nov 6, 2023

Conversation

Hardikl
Copy link
Contributor

@Hardikl Hardikl commented Nov 2, 2023

Added hidden cli in generate, which would fetched metrics details from ontap and update the map of description with panels and later update the dashboards from the map.
Note: This is applicable only for category 1 where panel contains only 1 expression.

./bin/harvest generate desc -p POLLER_NAME

cmd/tools/generate/generate.go Show resolved Hide resolved
cmd/tools/generate/generate.go Outdated Show resolved Hide resolved
cmd/tools/generate/generate.go Outdated Show resolved Hide resolved
if strings.Contains(expr, "/") || strings.Contains(expr, "*") || strings.Contains(expr, "+") || strings.Contains(expr, "-") {
// This indicates expressions with arithmetic operations, After adding appropriate description, this will be uncommented.
//t.Errorf(`dashboard=%s panel="%s" has many expressions`, dashPath, value.Get("title").String())
fmt.Printf(`dashboard=%s panel="%s" has many expressions \n`, dashPath, title)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we want t.errorf and not fmt.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, those are the cases for category 2 and 3.
t.errorf will be uncommented after the implementation of them later, else it will error out.

} else {
// This indicates table/timeseries with more than 1 expressions, After deciding next steps, this will be uncommented.
//t.Errorf(`dashboard=%s panel="%s" has many expressions`, dashPath, value.Get("title").String())
fmt.Printf(`dashboard=%s panel="%s" has many expressions \n`, dashPath, title)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we want t.errorf and not fmt.

Comment on lines +182 to +186
func VisitAllPanels(data []byte, handle func(path string, key gjson.Result, value gjson.Result)) {
visitPanels(data, "panels", "", handle)
}

func visitPanels(data []byte, panelPath string, pathPrefix string, handle func(path string, key gjson.Result, value gjson.Result)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we want to move these methods here? See if you can keep them in original file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Earlier it was in dashboard_test.go file, and need to use in other non-test files as well.
Thought would be non-test file should not use the method from test class, whereas reverse would be fine.

rahulguptajss
rahulguptajss previously approved these changes Nov 6, 2023
cmd/tools/generate/generate.go Outdated Show resolved Hide resolved
@cgrinds cgrinds merged commit 0e28291 into main Nov 6, 2023
10 checks passed
@cgrinds cgrinds deleted the hl_panel_desc_change_cli branch November 6, 2023 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants