Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[ML] Model management UI fixes and enhancements #79072

Merged
merged 7 commits into from
Oct 1, 2020
Merged

[ML] Model management UI fixes and enhancements #79072

merged 7 commits into from
Oct 1, 2020

Conversation

darnautov
Copy link
Contributor

Summary

Follow-up on #74978.

  • Fixes date formatting

  • Wraps JSON values in the EUI code block component
    image

  • "View training data" URL navigates to the results page with predefined is_training value in the query
    Oct-01-2020 10-13-49

  • Introduces "Edit" URL to the Ingest Pipeline edit page
    Oct-01-2020 10-17-33

Checklist

  • This renders correctly on smaller devices using a responsive layout. (You can test this in your browser

@darnautov darnautov added :ml v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Data Frame Analytics ML data frame analytics features v7.10.0 labels Oct 1, 2020
@darnautov darnautov requested a review from a team as a code owner October 1, 2020 08:18
@darnautov darnautov self-assigned this Oct 1, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

jgowdyelastic
jgowdyelastic reviewed Oct 1, 2020
View reviewed changes
x-pack/plugins/ml/common/util/time_formatter.ts Outdated
import { formatDate } from '@elastic/eui/lib/services/format';
import { TIME_FORMAT } from '../constants/time_format';

export const timeFormatter = (value: number) => {
Copy link
Member

Choose a reason for hiding this comment

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

this is very similar to the functions in https://github.com/elastic/kibana/blob/master/x-pack/plugins/ml/public/application/util/date_utils.ts

Also we have quite a few places that have almost identical functions to this, which are local to the file and not shared.

kibana/x-pack/plugins/ml/public/application/components/model_snapshots/model_snapshots_table.tsx

Line 249 in addbdf7

function renderDate(date: number) {

kibana/x-pack/plugins/ml/public/application/components/annotations/annotations_table/annotations_table.js

Line 380 in addbdf7

function renderDate(date) {

plus a handful of places where we're doing something like this:

kibana/x-pack/plugins/ml/public/application/data_frame_analytics/pages/analytics_management/components/models_management/expanded_row.tsx

Line 64 in addbdf7

value = formatDate(value, TIME_FORMAT);

I think it be worth moving date_utils.ts to common and merging with this file and then changing all uses of formatDate were it's being using with TIME_FORMAT
I don't know if all of that work would be too out of scope for this PR, if so, then maybe just the merging of date_utils.ts would be enough for now.

Copy link
Contributor Author

@darnautov darnautov Oct 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Refactored in 35b1c8c

  • Moved date_utils to common
  • Introduced a timeFormatter based on TIME_FORMAT in date_utils and replaced all duplicates

jgowdyelastic
jgowdyelastic approved these changes Oct 1, 2020
View reviewed changes
Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

Code LGTM

peteharverson
peteharverson approved these changes Oct 1, 2020
View reviewed changes
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested and LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

async chunks size

id before after diff
ml 10.5MB 10.5MB +4.3KB

distributable file count

id before after diff
default 45806 45807 +1

History

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

@darnautov darnautov merged commit 6caf6d5 into elastic:master Oct 1, 2020
phillipb added a commit to phillipb/kibana that referenced this pull request Oct 1, 2020
@phillipb
Merge branch 'master' of github.com:elastic/kibana into override-anom…
cfaef1d 
…aly-detection-partition-field

* 'master' of github.com:elastic/kibana: (76 commits)
  Fix z-index of KQL Suggestions dropdown (elastic#79184)
  [babel] remove unused/unneeded babel plugins (elastic#79173)
  [Search] Fix timeout upgrade link (elastic#79045)
  Always Show Embeddable Panel Header in Edit Mode (elastic#79152)
  [Ingest]: add more test for transform index (elastic#79154)
  [ML] DF Analytics: Collapsable sections on results pages (elastic#76641)
  [Fleet] Fix agent policy change action migration (elastic#79046)
  [Ingest Manager] Match package spec `dataset`->`data_stream` and `config_templates`->`policy_templates` renaming (elastic#78699)
  Revert "[Metrics UI] Add ability to override datafeeds and job config for partition field (elastic#78875)"
  [ML] Update transform cloning to include description and new fields (elastic#78364)
  chore(NA): remove non existing plugin paths from case api integration tests (elastic#79127)
  [Ingest Manager] Ensure we trigger agent policy updated event when we bump revision. (elastic#78836)
  [Metrics UI] Display No Data context.values as [NO DATA] (elastic#78038)
  [Monitoring] Missing data alert (elastic#78208)
  [Lens] Fix embeddable title and description for reporting and dashboard tooltip (elastic#78767)
  [Lens] Consistent Drag and Drop styles (elastic#78674)
  [ML] Model management UI fixes and enhancements (elastic#79072)
  [Metrics UI] Add ability to override datafeeds and job config for partition field (elastic#78875)
  [Security Solution]Fix basepath used by endpoint telemetry tests (elastic#79027)
  update rum agent version which contains longtasks (elastic#79105)
  ...
@darnautov darnautov mentioned this pull request Oct 2, 2020
Merged
darnautov added a commit to darnautov/kibana that referenced this pull request Oct 2, 2020
@darnautov
[ML] Model management UI fixes and enhancements (elastic#79072)
1404c6c 
* [ML] link to edit pipeline

* [ML] view training data link

* [ML] format stats and configs

* [ML] refactor date_utils

* [ML] fix types

* [ML] change "View" icon and label

* [ML] revert label change
@darnautov darnautov mentioned this pull request Oct 2, 2020
Closed
darnautov added a commit that referenced this pull request Oct 2, 2020
@darnautov
[ML] Model management UI fixes and enhancements (#79072) (#79254)
255e6a9 
* [ML] link to edit pipeline

* [ML] view training data link

* [ML] format stats and configs

* [ML] refactor date_utils

* [ML] fix types

* [ML] change "View" icon and label

* [ML] revert label change
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peteharverson peteharverson peteharverson approved these changes

@jgowdyelastic jgowdyelastic jgowdyelastic approved these changes

Assignees

@darnautov darnautov

Labels
Feature:Data Frame Analytics ML data frame analytics features :ml release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@darnautov @elasticmachine @kibanamachine @peteharverson @jgowdyelastic