Skip to content

Ingest UI Pattern Review Screen#5892

Merged
Bargs merged 25 commits intoelastic:feature/ingestfrom
Bargs:ingest/patternReview
Feb 4, 2016
Merged

Ingest UI Pattern Review Screen#5892
Bargs merged 25 commits intoelastic:feature/ingestfrom
Bargs:ingest/patternReview

Conversation

@Bargs
Copy link
Copy Markdown
Contributor

@Bargs Bargs commented Jan 13, 2016

Requires #5790. When reviewing this pull, you can ignore the changes from that PR which have been merged in.

Implements the pattern review step from #5974. Takes the pipeline and example docs from the previous step and creates an simple index pattern with a title, time_field_name (if a date field is present), and fields with auto detected types. The user can configure the index pattern we've generated. The result of the step is an index pattern object that can be sent to the new ingest API #5199 once the user clicks save (to be implemented in a coming PR).

This PR works with some hard coded test data which will be removed once all of the wizard steps have been merged into the feature/ingest branch and wired together.

@Bargs Bargs mentioned this pull request Jan 21, 2016
15 tasks
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

iw we got it wrong --> if we got it wrong?

@Bargs
Copy link
Copy Markdown
Contributor Author

Bargs commented Jan 27, 2016

Thanks for taking a look @michaelcheng429

@Bargs Bargs added the review label Feb 2, 2016
@jbudz
Copy link
Copy Markdown
Contributor

jbudz commented Feb 3, 2016

Sorting on the type column,
image

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.

should there be any target_field conflict checking on the front end? do you know if this is handled on the server?

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.

The ES ingest API simply overwrites the target_field if it already exists, so I don't think we have to worry about conflicts.

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 we escape these?

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.

Great catch, updated it and I think I covered everything

@jbudz
Copy link
Copy Markdown
Contributor

jbudz commented Feb 3, 2016

Looking good! Made some comments, passing back.

@jbudz jbudz assigned Bargs and unassigned jbudz Feb 3, 2016
@Bargs Bargs assigned jbudz and unassigned Bargs Feb 4, 2016
@Bargs
Copy link
Copy Markdown
Contributor Author

Bargs commented Feb 4, 2016

@jbudz made some updates and left some follow up comments, let me know how it looks now.

@jbudz
Copy link
Copy Markdown
Contributor

jbudz commented Feb 4, 2016

LGTM

@jbudz jbudz assigned Bargs and unassigned jbudz Feb 4, 2016
Bargs pushed a commit that referenced this pull request Feb 4, 2016
Ingest UI Pattern Review Screen
@Bargs Bargs merged commit c6bd928 into elastic:feature/ingest Feb 4, 2016
@tbragin tbragin added the v5.0.0 label Mar 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants