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.

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

[Transform] Show destination index mapping warning for the latest transform #87858

Merged
merged 4 commits into from
Jan 12, 2021

Conversation

darnautov
Copy link
Contributor

@darnautov darnautov commented Jan 11, 2021

Summary

With a recently revealed elasticsearch bug with the dynamic mappings, we need to show a warning callout in the UI.

image

@darnautov darnautov added :ml v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:Transforms ML transforms v7.11.0 v7.12.0 labels Jan 11, 2021
@darnautov darnautov requested a review from a team as a code owner January 11, 2021 15:45
@darnautov darnautov self-assigned this Jan 11, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

<p>
<FormattedMessage
id="xpack.transform.stepDetailsForm.destinationIndexWarning"
defaultMessage="Note that dynamic mappings based on the source fields will be used if the destination index does not exist. If alternate mappings are required, use the {docsLink} prior to starting the transform. If the transform fails, please check the messages tab for the transform on the Stack Management page for errors."
Copy link
Contributor

@lcawl lcawl Jan 11, 2021

Choose a reason for hiding this comment

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

Suggested change
defaultMessage="Note that dynamic mappings based on the source fields will be used if the destination index does not exist. If alternate mappings are required, use the {docsLink} prior to starting the transform. If the transform fails, please check the messages tab for the transform on the Stack Management page for errors."
defaultMessage="Before you start the transform, use index templates or the {docsLink} to ensure the mappings for your destination index match the source index. If the transform fails, check the messages tab on the Stack Management page for errors."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in 8bb38f5

@walterra
Copy link
Contributor

Technically the callout seems to be horizontally aligned with the Destination index form row, but it still looks a bit off being the only item in another column next to the form items and with an offset for the input element.

Here's an alternative version, it will consume more vertical space though, but it's still in line with what the EUI docs suggest (https://elastic.github.io/eui/#/forms/form-validation), what do you think?

image

@darnautov
Copy link
Contributor Author

@walterra layout has been changed in 8bb38f5

@darnautov darnautov requested a review from lcawl January 11, 2021 17:38
Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

Great, LGTM!

@peteharverson peteharverson added the bug Fixes for quality problems that affect the customer experience label Jan 11, 2021
Copy link
Contributor

@przemekwitek przemekwitek left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks!

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.

Text changes LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
transform 1.0MB 1.0MB +1.4KB

History

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

@darnautov darnautov merged commit 83e8fe8 into elastic:master Jan 12, 2021
@darnautov darnautov deleted the transform-latest-warning branch January 12, 2021 12:03
darnautov added a commit to darnautov/kibana that referenced this pull request Jan 12, 2021
…nsform (elastic#87858)

* [Transform] show callout for the latest transform

* [Transform] fix flex styles

* [Transform] change to vertical layout, update warning message

* [Transform] fix messages
darnautov added a commit to darnautov/kibana that referenced this pull request Jan 12, 2021
…nsform (elastic#87858)

* [Transform] show callout for the latest transform

* [Transform] fix flex styles

* [Transform] change to vertical layout, update warning message

* [Transform] fix messages
darnautov added a commit that referenced this pull request Jan 12, 2021
…nsform (#87858) (#87998)

* [Transform] show callout for the latest transform

* [Transform] fix flex styles

* [Transform] change to vertical layout, update warning message

* [Transform] fix messages
darnautov added a commit that referenced this pull request Jan 12, 2021
…nsform (#87858) (#87999)

* [Transform] show callout for the latest transform

* [Transform] fix flex styles

* [Transform] change to vertical layout, update warning message

* [Transform] fix messages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Transforms ML transforms :ml release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants