[ML] DF Analytics creation wizard: show link to results #74025
Conversation
alvarezmelissa87
added
release_note:enhancement
:ml
Feature:Data Frame Analytics
|
Pinging @elastic/ml-ui (:ml) |
alvarezmelissa87
force-pushed
the
ml-dfanalytics-wizard-results-link
branch
from
47c6391
to
cf8ed73
Compare
| return ( | ||
| <Fragment> | ||
| <EuiCard | ||
| // @ts-ignore |
There was a problem hiding this comment.
nit: would be good to comment why we need this ts-ignore
There was a problem hiding this comment.
Moved the inline style to a separate file so this ignore isn't needed anymore. d12ebe4
| const [initialized, setInitialized] = useState<boolean>(false); | ||
| const [failedJobMessage, setFailedJobMessage] = useState<string | undefined>(undefined); | ||
| const [jobFinished, setJobFinished] = useState<boolean>(false); | ||
| const [currentProgress, setCurrentProgress] = useState< | ||
| | { | ||
| currentPhase: number; |
There was a problem hiding this comment.
this should probably have a proper type which can also be used in ProgressStats, where currently any is currently being used.
There was a problem hiding this comment.
Updated in d12ebe4
|
|
||
| interface Props { | ||
| currentProgress: any; | ||
| failedJobMessage: any; |
There was a problem hiding this comment.
looks like this can be string | undefined
There was a problem hiding this comment.
Good spot - updated in d12ebe4
| return ( | ||
| <Fragment> | ||
| <EuiCard | ||
| // @ts-ignore |
There was a problem hiding this comment.
why is this ts-ignore needed? a comment here would be helpful
There was a problem hiding this comment.
Moved the inline style to a separate file so this ignore isn't needed anymore. d12ebe4
| @@ -130,6 +130,6 @@ export function isCompletedAnalyticsJob(stats: DataFrameAnalyticsStats) { | |||
| return stats.state === DATA_FRAME_TASK_STATE.STOPPED && progress === 100; | |||
| } | |||
|
|
|||
| export function getResultsUrl(jobId: string, analysisType: string) { | |||
| export function getResultsUrl(jobId: string, analysisType: ANALYSIS_CONFIG_TYPE | string) { | |||
There was a problem hiding this comment.
nit, and probably outside the scope of this PR, but it would be good if this only took ANALYSIS_CONFIG_TYPE
it looks like getAnalysisType should return a ANALYSIS_CONFIG_TYPE rather than a string.
There was a problem hiding this comment.
Yeah I noticed that, too, but it will involve touching more files so can be done in a separate PR.
|
This has been updated and is ready for a final look when you get a chance. cc @jgowdyelastic, @qn895 |
|
Tested and LGTM 💯 |
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
page load bundle size
History
To update your PR or re-run it, just comment with: |
* show view results card once job complete * update types * update types and move css to own file
* master: [ML] DF Analytics creation wizard: show link to results (elastic#74025)
Summary
Related meta issue: #66661
Add view results link next to analytics list link at the end of the creation wizard. Only show the link to the results if the job completed successfully.
When job is still running:
Once job is finished:
Checklist
Delete any items that are not applicable to this PR.