Skip to content

Change type of text fields to keyword#10542

Merged
kvch merged 3 commits intoelastic:masterfrom
kvch:fix-journalbeat-make-texts-keywords
Feb 5, 2019
Merged

Change type of text fields to keyword#10542
kvch merged 3 commits intoelastic:masterfrom
kvch:fix-journalbeat-make-texts-keywords

Conversation

@kvch
Copy link
Copy Markdown
Contributor

@kvch kvch commented Feb 4, 2019

@kvch kvch requested a review from a team as a code owner February 4, 2019 19:07
@kvch kvch mentioned this pull request Feb 4, 2019
Closed
@kvch
Copy link
Copy Markdown
Contributor Author

kvch commented Feb 4, 2019

jenkins test this

@kvch kvch changed the title Change type of text fileds to keyword Change type of text fields to keyword Feb 4, 2019
webmat
webmat approved these changes Feb 5, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@webmat webmat 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!

Any idea why only 2/7 of the fields end up changing in the asciidoc? Are the other ones (which are nested deeper) skipped because there's too many?

@ruflin
Copy link
Copy Markdown
Contributor

ruflin commented Feb 5, 2019

Very good point by @webmat I assume something is wrong with the indentation or naming in the fields.yml which means all the other fields are not part of the template either.

@kvch
Copy link
Copy Markdown
Contributor Author

kvch commented Feb 5, 2019 •
edited
Loading

@ruflin Indeed. I've fixed the indentation problem. Now all required fields are changed.

ruflin
ruflin approved these changes Feb 5, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Sounds like we probably had the indentation issue already in 6.x which would be a bug. Could you backport this also to 6.6 and 6.x?

@kvch
Copy link
Copy Markdown
Contributor Author

kvch commented Feb 5, 2019

Sure.

@kvch kvch added the needs_backport PR is waiting to be backported to other branches. label Feb 5, 2019
@kvch
Copy link
Copy Markdown
Contributor Author

kvch commented Feb 5, 2019

Opened two backports:

webmat
webmat approved these changes Feb 5, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

LGTM

@kvch kvch force-pushed the fix-journalbeat-make-texts-keywords branch from 440abd0 to be8ba58 Compare February 5, 2019 21:17
@kvch
Copy link
Copy Markdown
Contributor Author

kvch commented Feb 5, 2019

Rebased

webmat
webmat approved these changes Feb 5, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

Still LGTM ;-)

@kvch
Copy link
Copy Markdown
Contributor Author

kvch commented Feb 5, 2019

Travis refuses to start, so I am waiting for jenkins.

@kvch kvch force-pushed the fix-journalbeat-make-texts-keywords branch from be8ba58 to c44de40 Compare February 5, 2019 21:52
@kvch
Copy link
Copy Markdown
Contributor Author

kvch commented Feb 5, 2019

All relevant tests are green.

@kvch kvch merged commit 3b20a57 into elastic:master Feb 5, 2019
DStape pushed a commit to DStape/beats that referenced this pull request Aug 20, 2019
@kvch
Change type of text fields to keyword (elastic#10542)
89dc1b8
@kvch kvch removed the needs_backport PR is waiting to be backported to other branches. label Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ruflin ruflin ruflin approved these changes

+2 more reviewers

@webmat webmat webmat approved these changes

@urso urso urso approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Journalbeat review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kvch @ruflin @webmat @urso