-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Add time range parameters to labels API #2964
Conversation
f5773fe
to
bc9005d
Compare
This pr is ready for review! |
bc9005d
to
c6661b4
Compare
0da6da5
to
91d8a5c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing, good work! 💪 It's perfect really just one comment (:
pkg/api/query/v1.go
Outdated
if err != nil { | ||
return nil, nil, &api.ApiError{Typ: api.ErrorBadData, Err: err} | ||
} | ||
// Prometheus doesn't check this, do we need to? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds useful, no? (:
Please ask comments on PR itself not in comment (: So we can still merge PR if code is ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should and it's probably worth contributing such a check upstream IMHO
@@ -144,6 +144,10 @@ message LabelNamesRequest { | |||
|
|||
// TODO(bwplotka): Move Thanos components to use strategy instead. Including QueryAPI. | |||
PartialResponseStrategy partial_response_strategy = 2; | |||
|
|||
int64 start = 3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤙
Signed-off-by: Ben Ye <yb532204897@gmail.com>
91d8a5c
to
2d2cda6
Compare
I have addressed the comment. Please take a look again. 😄 |
2d2cda6
to
acd0a5c
Compare
Signed-off-by: Ben Ye <yb532204897@gmail.com>
acd0a5c
to
9a92e23
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Signed-off-by: Ben Ye yb532204897@gmail.com
Changes
Fixes: #1811
Verification