[ML] Changes group-by and agg names. #36686
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
walterra
added
:ml
release_note:skip
|
Pinging @elastic/ml-ui |
💔 Build Failed |
💔 Build Failed |
💚 Build Succeeded |
jgowdyelastic
approved these changes
May 21, 2019
| field: fieldName, | ||
| interval: '1m', | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| const illegalEsAggNameChars = /[[\]>]/g; |
There was a problem hiding this comment.
i would have thought there are a lot more illegal characters for an aggregation name, is there something significant about these ones?
There was a problem hiding this comment.
I was surprised too. Originally it was [a-zA-Z0-9\\-_], here's the reference to the elasticsearch PR that changed this: elastic/elasticsearch#6708
peteharverson
approved these changes
May 21, 2019
There was a problem hiding this comment.
Tested and LGTM.
Don't suspect it's related to this PR, but the data frame I created when testing this out is stuck at 99%
elasticsearch.log doesn't show any errors:
[2019-05-21T09:55:59,904][INFO ][o.e.x.d.t.DataFrameTransformPersistentTasksExecutor] [DESKTOP-RQG9008] Successfully completed and scheduled task in node operation
[2019-05-21T09:56:01,880][WARN ][o.e.x.c.i.AsyncTwoPhaseIndexer] [DESKTOP-RQG9008] Schedule was triggered for job [nginx_bytes_df], but prior indexer is still running (with state [INDEXING]
[2019-05-21T09:56:01,947][INFO ][o.e.x.d.t.DataFrameTransformTask] [DESKTOP-RQG9008] Finished indexing for data frame transform [nginx_bytes_df] checkpoint [1]
walterra
added a commit
to walterra/kibana
that referenced
this pull request
May 21, 2019
So far the groupby/agg name shown in the dropdowns and the name used for the ES config agg name has been the same. This PR changes the behaviour to the following: - group_by aggregation names will be left as original e.g. clientip, timestamp - agg functions will be added to the end as a nested object e.g. doc.bytes.sent.sum and doc.bytes.sent.avg and duration.max - the drop down display labels in the pickers in the wizard will continue to use the sql-like notation e.g. date_histogram(timestamp), terms(clientip), sum(doc.bytes.sent), max(duration) - when auto-generating aggregation names, unsupported characters ([]>) will be removed.
walterra
added a commit
that referenced
this pull request
May 21, 2019
So far the groupby/agg name shown in the dropdowns and the name used for the ES config agg name has been the same. This PR changes the behaviour to the following: - group_by aggregation names will be left as original e.g. clientip, timestamp - agg functions will be added to the end as a nested object e.g. doc.bytes.sent.sum and doc.bytes.sent.avg and duration.max - the drop down display labels in the pickers in the wizard will continue to use the sql-like notation e.g. date_histogram(timestamp), terms(clientip), sum(doc.bytes.sent), max(duration) - when auto-generating aggregation names, unsupported characters ([]>) will be removed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
So far the groupby/agg name shown in the dropdowns and the name used for the ES config agg name has been the same.
This PR changes the behaviour to the following:
group_byaggregation names will be left as original e.g.clientip,timestampdoc.bytes.sent.sumanddoc.bytes.sent.avgandduration.maxdate_histogram(timestamp),terms(clientip),sum(doc.bytes.sent),max(duration)[]>) will be removed.Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.This was checked for cross-browser compatibility, including a check against IE11No DOM related changes.Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportDocumentation was added for features that require explanation or tutorialsThis was checked for keyboard-only and screenreader accessibility