-
Notifications
You must be signed in to change notification settings - Fork 45
Adds rollup metadata service that deals with initializing and crud operations #337
Conversation
…erations for the a rollup metadata
Codecov Report
@@ Coverage Diff @@
## master #337 +/- ##
============================================
- Coverage 71.37% 71.30% -0.07%
- Complexity 1195 1219 +24
============================================
Files 181 181
Lines 6249 6448 +199
Branches 954 984 +30
============================================
+ Hits 4460 4598 +138
- Misses 1314 1361 +47
- Partials 475 489 +14
Continue to review full report at Codecov.
|
// TODO: Could make this an extension function of DateHistogram and add to some utility file | ||
private fun getRoundingStrategy(dateHistogram: DateHistogram): Rounding { | ||
val intervalString = (dateHistogram.calendarInterval ?: dateHistogram.fixedInterval) as String | ||
// TODO: Make sure the interval string is validated before getting here so we don't get errors |
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.
Let's double check this to confirm when is the earliest point in the code that we catch an incorrect DateHistogram interval string. If it isn't caught before here, we would enter the else
statement and may not handle the case correctly.
Perhaps we could leverage the DateHistogram builder and try to enter the given interval string and catch any exceptions that are thrown. If this approach is valid, we could even put it in the init
of Rollup
to validate early.
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.
Will cover TODOs and any cleanups needed in follow up PRs to unblock the PRs that depend on these.
…erations for the a rollup metadata
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.