Read-only table of rollup jobs #21900
Conversation
cjcenizal
commented
Aug 11, 2018
•
cjcenizal
commented
cjcenizal
force-pushed
the
rollup-jobs-table
branch
from
023d700 to
4d376f8
Compare
💔 Build Failed |
cjcenizal
force-pushed
the
rollup-jobs-table
branch
from
4d376f8 to
6af27a5
Compare
💔 Build Failed |
💔 Build Failed |
|
|
|
@yaronp68 Date histogram is a required grouping for rollup jobs. I left it out of the "Groups" column in the table because I figured it'd be repetitive and redundant. I'm a little on the fence about surfacing it as its own grouping in the detail panel. It's currently visible in the Summary tab. What do you think? Do you think there's a better way to present this information? |
|
|
||
| import { | ||
| TabSummary, | ||
| TabTerms, |
There was a problem hiding this comment.
Histogram tab is missing?
There was a problem hiding this comment.
From looking at the docs, I think the "histogram" group is represented by the terms field (https://www.elastic.co/guide/en/elasticsearch/reference/master/rollup-put-job.html). What do you think of referring to this Histogram in the UI (instead of Terms as it currently is), and add a note that it's represented by terms in the API?
There was a problem hiding this comment.
Never mind! My mistake. You're right. I'll add Histogram in addition to the existing Terms.
| time_zone: dateHistogramTimeZone, | ||
| field: dateHistogramField, | ||
| }, | ||
| terms = { |
There was a problem hiding this comment.
histogram is missing here
There was a problem hiding this comment.
Good catch, done!
| groups: { | ||
| date_histogram: { | ||
| interval: rollupInterval, | ||
| delay: rollupDelay, |
There was a problem hiding this comment.
delay config is optional, so it's possible that this is empty, which looks kinda weird in UI. maybe we can default to None either right here or in the table and flyout with {{rollupDelay || 'None'}}?
| </EuiDescriptionListTitle> | ||
|
|
||
| <EuiDescriptionListDescription> | ||
| {rollupIndex} |
There was a problem hiding this comment.
long values break to next line, maybe we can just increase the flyout size, especially since Histogram tab is needed
| </EuiDescriptionListDescription> | ||
|
|
||
| <EuiDescriptionListTitle> | ||
| <strong>Cron</strong> |
There was a problem hiding this comment.
it may be helpful to include tooltip(s) about the distinction between Cron (how often the job runs) and Interval (date histogram bucket interval)
| {rollupCron} | ||
| </EuiDescriptionListDescription> | ||
|
|
||
| <EuiDescriptionListTitle> |
There was a problem hiding this comment.
Consider putting Delay, Interval, Time field, and Timezone under a Date Histogram title, I think that will correlate this info with the user's configuration more strongly. I think Time field should also be the first piece of info in this block since it's the most important
There was a problem hiding this comment.
Great idea, done.
|
|
||
| esSection.register('index_rollup_jobs', { | ||
| visible: true, | ||
| display: 'Index Rollup Jobs', |
There was a problem hiding this comment.
can this just be called Rollup Jobs? I haven't seen rollups prepended with Index in any docs
💔 Build Failed |
|
@yaronp68 Cell content is now truncated:
|
|
@jen-huang All comments addressed, ready for your 👀 again. |
💔 Build Failed |
💔 Build Failed |
| fieldName: 'rollupIndex', | ||
| }, { | ||
| name: 'Delay', | ||
| fieldName: 'rollupDelay', |
There was a problem hiding this comment.
this could use a custom render to show None for no value like in the flyout:
render(rollupDelay) => <Fragment>{rollupDelay || 'None'}</Fragment>
