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

RuleGroups.Validate(...) needs unexported struct ruleGroups as input #7128

Closed
sylr opened this issue Apr 15, 2020 · 13 comments
Closed

RuleGroups.Validate(...) needs unexported struct ruleGroups as input #7128

sylr opened this issue Apr 15, 2020 · 13 comments

Comments

@sylr
Copy link
Contributor

sylr commented Apr 15, 2020

I'm trying to upgrade prometheus dep in thanos and I'm stuck with this:

$ make build
all modules verified
>> building binaries /Users/s.rabot/go/bin
 >   thanos
# github.com/thanos-io/thanos/cmd/thanos
cmd/thanos/check.go:88:29: not enough arguments in call to promRgs.Validate
	have ()
	want (rulefmt.ruleGroups)

func (g *RuleGroups) Validate(node ruleGroups) (errs []error) {

cc @bwplotka

@bwplotka
Copy link
Member

Why you put issue on Prometheus for this? It's normal we will have build errors to fix. We plan to upgrade deps after chunk iterator.

@sylr
Copy link
Contributor Author

sylr commented Apr 15, 2020

Because I thought there was little that could be done in thanos if prometheus does not export ruleGroups or adds a NewruleGroups() ruleGroups func.

Am I mistaken ?

@sylr sylr changed the title RuleGroups.Validate(...) need unexported struct ruleGroups as input RuleGroups.Validate(...) needs unexported struct ruleGroups as input Apr 16, 2020
@aclowkey
Copy link

aclowkey commented Apr 19, 2020

I think it should be exported for non-thanos related reasons.
For example if I want to build a tool that manages alerts, and not implement my own validation logic, I can't.

I could do something like this:

_, errs := rulefmt.Parse([]byte(definition)

Then errs would have elements of type rulefmt.Error which have WrappedError that has needed context, but all of it's fields are unexported..

@brian-brazil
Copy link
Contributor

Use of our internals by 3rd parties is unsupported, but we will generally accept PRs to make things public.

@bwplotka
Copy link
Member

bwplotka commented May 4, 2020

Let's move forward with this then... (:

It's blocking thanos-io/thanos#2535

@bwplotka
Copy link
Member

bwplotka commented May 4, 2020

Are you still happy to continue this @sylr

@sylr
Copy link
Contributor Author

sylr commented May 4, 2020

@bwplotka I wish I could but I am pretty busy at the moment.

@roidelapluie
Copy link
Member

Actually the way forward is to get back to yaml.v2 as there are missing features in yaml.v3

@bwplotka
Copy link
Member

bwplotka commented May 4, 2020

Do you have any details / related discussion @roidelapluie ?

@sylr
Copy link
Contributor Author

sylr commented May 4, 2020

#7142 (comment)

@roidelapluie
Copy link
Member

Actually things are moving now.. I will see that later on.

roidelapluie added a commit to roidelapluie/prometheus that referenced this issue May 5, 2020
Fixes prometheus#7128

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
@roidelapluie
Copy link
Member

cmd/thanos/check.go > I do not see that file in thanos codebase?

@bwplotka
Copy link
Member

bwplotka commented May 6, 2020 via email

@sylr sylr closed this as completed Apr 27, 2023
@prometheus prometheus locked as resolved and limited conversation to collaborators Oct 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants