Skip to content

Conversation

@cruvie
Copy link

@cruvie cruvie commented Aug 25, 2023

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Add CheckRulesFiles method to replace rulefmt.ParseFile in prometheus
#6641

Verification

run TestCheckRulesFiles to text the method

@cruvie cruvie changed the title "rule" Add CheckRulesFiles method to replace rulefmt.ParseFile in prometheus #6641 "rule" Add CheckRulesFiles method to replace rulefmt.ParseFile in prometheus https://github.com/thanos-io/thanos/issues/6641 Aug 25, 2023
@cruvie cruvie changed the title "rule" Add CheckRulesFiles method to replace rulefmt.ParseFile in prometheus https://github.com/thanos-io/thanos/issues/6641 "rule" Add CheckRulesFiles method to replace rulefmt.ParseFile in prometheus Aug 25, 2023

import (
"context"
"github.com/thanos-io/thanos/pkg/errutil"
Copy link
Contributor

Choose a reason for hiding this comment

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

can you group this with the other thanos-io imports?

Copy link
Author

Choose a reason for hiding this comment

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

do you mean

import (
		"github.com/thanos-io/thanos/pkg/errutil"
)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, no i meant to put it on line 20 roughly where rules/rulespb and tracing are imported

Copy link
Author

Choose a reason for hiding this comment

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

of course

return srv.ctx
}

func CheckRulesFiles(filePaths []string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont see that this is called anywhere; why is it needed?

Copy link
Author

Choose a reason for hiding this comment

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

just like the issue I mentioned above, we need a method to replace rulefmt.ParseFile in prometheus since thanos change the alert rule file and and rulefmt.ParseFile cannot chech the changed file.

Copy link
Member

Choose a reason for hiding this comment

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

Is there any way we could use it in the code-base itself? I'm afraid that this logic will get stale.

Copy link
Author

Choose a reason for hiding this comment

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

no, the key func is ValidateAndCount(), which is used by thanos CLI tool,too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants