Skip to content

[ML] Data Frame Analytics: Use EuiDataGrid for outlier result page - #58235

Merged
walterra merged 16 commits into
elastic:masterfrom
walterra:ml-data-frame-analytics-data-grid
Mar 25, 2020
Merged

[ML] Data Frame Analytics: Use EuiDataGrid for outlier result page#58235
walterra merged 16 commits into
elastic:masterfrom
walterra:ml-data-frame-analytics-data-grid

Conversation

@walterra

@walterra walterra commented Feb 21, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Part of #51288.

  • Replaces EuiInMemoryTable with EuiDataGrid
  • Replaces the memory table's search with QueryInputFilter

image

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@walterra walterra self-assigned this Feb 21, 2020
@walterra walterra changed the title [ML] Use EuiDataGrid for data frame analytics results pages [ML] Data Frame Analytics: Use EuiDataGrid for results pages Mar 12, 2020
@walterra
walterra force-pushed the ml-data-frame-analytics-data-grid branch from 4fe918a to f0f2553 Compare March 13, 2020 12:18
@walterra walterra changed the title [ML] Data Frame Analytics: Use EuiDataGrid for results pages [ML] Data Frame Analytics: Use EuiDataGrid for outlier result page Mar 13, 2020
@walterra
walterra force-pushed the ml-data-frame-analytics-data-grid branch from 8320039 to d8fb346 Compare March 13, 2020 17:49
@walterra
walterra marked this pull request as ready for review March 13, 2020 17:49
@walterra
walterra requested review from a team as code owners March 13, 2020 17:49
@elasticmachine

elasticmachine commented Mar 13, 2020

Copy link
Copy Markdown
Contributor

Pinging @elastic/ml-ui (:ml)

peteharverson
peteharverson reviewed Mar 16, 2020
View reviewed changes
Comment thread ...ytics/pages/analytics_exploration/components/exploration_query_bar/exploration_query_bar.tsx Outdated
setSearchQuery: Dispatch<SetStateAction<SavedSearchQuery>>;
}

const QUERY_LANGUAGE_KUERY = 'kuery';

@peteharverson peteharverson Mar 16, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use the enums out of ml/common/constants/search.ts here?

@peteharverson

peteharverson commented Mar 16, 2020

Copy link
Copy Markdown
Contributor

I am seeing a few issues when I use the paging controls (change page, or change page size).

For example, with the glass_outlier_detection_1 job from the MLQA dev bootstrap analytics suite, if I move from page 1 to 2, and then switch the page size to 50 I see:

image

And for an outlier job on a cloudwatch transform, switching from the first page to the last page of results, the page goes blank with the error:

image

@peteharverson

peteharverson commented Mar 16, 2020

Copy link
Copy Markdown
Contributor

For the search bar, it would be nice to propagate any syntax errors to the user, as we do in the data visualizer for example:

image

Currently the error is only seen in the browser console:

image

@alvarezmelissa87

alvarezmelissa87 commented Mar 16, 2020

Copy link
Copy Markdown
Contributor

I'm getting the same errors that @peteharverson has commented about.

@walterra walterra mentioned this pull request Mar 20, 2020
Closed
11 tasks done
alvarezmelissa87
alvarezmelissa87 reviewed Mar 24, 2020
View reviewed changes
Comment thread x-pack/plugins/ml/public/application/data_frame_analytics/common/fields.ts
return docs.some(row => row._source[k] !== null);
})
.slice(0, MAX_COLUMNS);
return docs.some(row => row._source[k] !== null);

@alvarezmelissa87 alvarezmelissa87 Mar 24, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the removed .slice() here - do we not need to limit to a particular number of columns initially anymore?

@walterra walterra Mar 24, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I plan to re-introduce that in a follow-up, beginning with transforms we thought it's fine, but for indices like filebeat data grid has a hard time handling the load.

alvarezmelissa87
alvarezmelissa87 reviewed Mar 24, 2020
View reviewed changes
Comment thread ...analytics/pages/analytics_exploration/components/outlier_exploration/outlier_exploration.tsx
import { ExplorationDataGrid } from '../exploration_data_grid';
import { ExplorationQueryBar } from '../exploration_query_bar';

const FEATURE_INFLUENCE = 'feature_influence';

@alvarezmelissa87 alvarezmelissa87 Mar 24, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also defined in exploration_data_grid.tsx - perhaps this could be moved to a shared location and imported into both places it's used?

alvarezmelissa87
alvarezmelissa87 reviewed Mar 24, 2020
View reviewed changes

@alvarezmelissa87 alvarezmelissa87 left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM overall - just going to give it a local test before I approve 😄
Feel free to merge if you get another green check mark before I'm online tomorrow 👌

@walterra

walterra commented Mar 25, 2020

Copy link
Copy Markdown
Contributor Author

Fixed: Propagating query string error, cell value accessors, paging.

@kibanamachine

kibanamachine commented Mar 25, 2020

Copy link
Copy Markdown
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@walterra
walterra merged commit caaeb37 into elastic:master Mar 25, 2020
@walterra
walterra deleted the ml-data-frame-analytics-data-grid branch March 25, 2020 06:29
@walterra walterra mentioned this pull request Mar 25, 2020
Merged
walterra added a commit that referenced this pull request Mar 25, 2020
@walterra
[ML] Data Frame Analytics: Use EuiDataGrid for outlier result page (#…
aae59b1 
…58235) (#61212)

- Replaces EuiInMemoryTable with EuiDataGrid
- Replaces the memory table's search with QueryInputFilter
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
@walterra
[ML] Data Frame Analytics: Use EuiDataGrid for outlier result page (e…
445e872 
…lastic#58235)

- Replaces EuiInMemoryTable with EuiDataGrid
- Replaces the memory table's search with QueryInputFilter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alvarezmelissa87 alvarezmelissa87 alvarezmelissa87 left review comments
@peteharverson peteharverson peteharverson left review comments
@jgowdyelastic jgowdyelastic Awaiting requested review from jgowdyelastic
@darnautov darnautov Awaiting requested review from darnautov

Assignees

@walterra walterra

Labels

Feature:Data Frame Analytics ML data frame analytics features :ml release_note:enhancement v7.7.0 v8.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@walterra @elasticmachine @peteharverson @alvarezmelissa87 @kibanamachine