Skip to content

Conversation

@jgowdyelastic
Copy link
Member

@jgowdyelastic jgowdyelastic commented Oct 25, 2018

Fixes issues with the displaying of fields.

  • if a csv file has a column title but no values for that column, the field type can't be discovered and the card would display with a missing header.
  • the message field in semi structured text files is missing from field_stats and so are then missing from the field list.

Also tweaks the style of the experimental badge to make it smaller and less intrusive.

@jgowdyelastic jgowdyelastic added bug Fixes for quality problems that affect the customer experience review non-issue Indicates to automation that a pull request should not appear in the release notes v7.0.0 :ml v6.5.0 v6.6.0 labels Oct 25, 2018
@jgowdyelastic jgowdyelastic self-assigned this Oct 25, 2018
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui

return field;
} else {
// field is not in the field stats
// this could be a text field or a field which the endpoint has

Choose a reason for hiding this comment

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

It should be "this could be the message field for a semi-structured log file or a field which the endpoint has"

return {
name: fName,
name: name,
type,
Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 Oct 25, 2018

Choose a reason for hiding this comment

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

Nit 🕸 could be just name

Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

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.

LGTM

@jgowdyelastic jgowdyelastic changed the title [Ml] Fixing results for known fields [Ml] Fixing results for unknown fields Oct 25, 2018
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.

One question. Otherwise LGTM

EuiBetaBadge,
} from '@elastic/eui';

export function ExperimentalBadge({ tooltipContent }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you apply a className directly to EuiBetaBadge to achieve the same result here?

Copy link
Member Author

Choose a reason for hiding this comment

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

the css would need to be in a shared location. so i decided it would be easier to just create a shared component instead to make it obvious that it's being overridden.

@peteharverson peteharverson changed the title [Ml] Fixing results for unknown fields [ML] Fixing results for unknown fields Oct 25, 2018
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

jgowdyelastic added a commit to jgowdyelastic/kibana that referenced this pull request Oct 30, 2018
* [Ml] Fixing results for known fields

* renaming known-type to unknown

* correcting comments

* tiny refactor
jgowdyelastic added a commit that referenced this pull request Oct 30, 2018
* [Ml] Fixing results for known fields

* renaming known-type to unknown

* correcting comments

* tiny refactor
jgowdyelastic added a commit to jgowdyelastic/kibana that referenced this pull request Oct 31, 2018
* [Ml] Fixing results for known fields

* renaming known-type to unknown

* correcting comments

* tiny refactor
jgowdyelastic added a commit that referenced this pull request Oct 31, 2018
* [Ml] Fixing results for known fields

* renaming known-type to unknown

* correcting comments

* tiny refactor
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 :ml non-issue Indicates to automation that a pull request should not appear in the release notes review v6.5.0 v6.6.0 v7.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants