-
Notifications
You must be signed in to change notification settings - Fork 7
refactor: expose a way of ordering explorer query by interval #74
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
Conversation
Codecov Report
@@ Coverage Diff @@
## main #74 +/- ##
============================================
- Coverage 28.02% 22.69% -5.33%
- Complexity 48 66 +18
============================================
Files 43 64 +21
Lines 1092 1586 +494
Branches 34 49 +15
============================================
+ Hits 306 360 +54
- Misses 780 1219 +439
- Partials 6 7 +1
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.
|
||
private Maybe<OrderByExpression> buildAttributeOrderExpression(ExploreOrderArgument argument) { | ||
return Maybe.fromOptional(argument.attribute()) | ||
.doOnComplete(() -> log.error("Attribute order argument missing attribute: {} ", argument)) |
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.
RxJava has weird operator names. doOnComplete
should ideally have been called doOnEmpty
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.
yeah I had to double check that one - complete == an empty final state, success == a final state with a value.
Description
Because the interval field in explorer was first class, the generic ordering argument that accepted a string as key does not support anything beyond the default ordering, which is ascending. Along with hypertrace/gateway-service#82 , this adds support for ordering by interval by passing the name of the interval field
intervalStart
without otherwise changing the api.Testing
Added UT. Will test e2e before merging
Checklist: