-
Notifications
You must be signed in to change notification settings - Fork 10
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
Metrics followup #123
Metrics followup #123
Conversation
Codecov Report
@@ Coverage Diff @@
## main #123 +/- ##
============================================
- Coverage 81.21% 81.11% -0.11%
+ Complexity 573 569 -4
============================================
Files 53 53
Lines 2204 2192 -12
Branches 234 235 +1
============================================
- Hits 1790 1778 -12
Misses 319 319
Partials 95 95
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
...rc/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestToPromqlConverter.java
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@rish691 As part of this PR, can you put the constraints on log4j related library coming from a third party to use the latest |
@@ -125,12 +123,12 @@ public OkHttpResponseCallback(Request request) { | |||
} | |||
|
|||
@Override | |||
public void onResponse(Call call, Response response) throws IOException { | |||
public void onResponse(@NonNull Call call, @NonNull Response response) { |
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.
why do we need explicitly @NonNull
?
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.
IDE suggestion, it says Not annotated parameter overrides @NotNull parameter
, basically this param is marked not null in the parent
...rc/main/java/org/hypertrace/core/query/service/prometheus/PrometheusBasedRequestHandler.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/hypertrace/core/query/service/prometheus/QueryRequestToPromqlConverter.java
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ValueCase valueCase = expression.getValueCase(); | ||
switch (valueCase) { |
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.
nit
ValueCase valueCase = expression.getValueCase(); | |
switch (valueCase) { | |
switch (expression.getValueCase()) { |
From #119