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

add ignore_empty_value parameter in set ingest processor #57030

Merged
merged 9 commits into from
Jun 15, 2020

Conversation

gaobinlong
Copy link
Contributor

@gaobinlong gaobinlong commented May 21, 2020

Relates to #54783.
The main point of this PR is adding ignore_empty_value in set ingest processr, which can exit quitely if the templated value is null or emtpy string when the parameter is set to true.

Because the template value like {{foo}} produces empty string when the field foo does not exist, is null or empty string, so the parameter ignore_empty_value ignores both nullor empty string when it's set to true.

@cbuescher cbuescher added the :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP label May 25, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Ingest)

@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label May 25, 2020
@danhermann danhermann self-requested a review May 28, 2020 14:56
@danhermann
Copy link
Contributor

@elasticmachine ok to test

@danhermann
Copy link
Contributor

@elasticmachine update branch

Copy link
Contributor

@danhermann danhermann left a comment

Choose a reason for hiding this comment

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

@gaobinlong, this looks great. I left two minor changes below and I'll get it merged if those look good to you.

docs/reference/ingest/processors/set.asciidoc Outdated Show resolved Hide resolved
gaobinlong and others added 3 commits June 2, 2020 21:24
Co-authored-by: Dan Hermann <danhermann@users.noreply.github.com>
Co-authored-by: Dan Hermann <danhermann@users.noreply.github.com>
@danhermann
Copy link
Contributor

@elasticmachine update branch

@danhermann
Copy link
Contributor

@gaobinlong, sorry for the delay on this one. If you can fix the two new compile errors (just add another boolean parameter to the createSetProcessor method in the new tests testSetMetadataIfSeqNo and testSetMetadataIfPrimaryTerm), I will get in merged in promptly so there won't be any more conflicts on it.

@gaobinlong
Copy link
Contributor Author

@danhermann, I have done that and all checks have passed.

@danhermann
Copy link
Contributor

Thank you, @gaobinlong! I've merged this and will also get it back-ported to the appropriate branches.

@P1llus
Copy link
Member

P1llus commented Jun 25, 2020

@gaobinlong Just wanted to say thank you for the contribution, I had a issue open on this for a while and it allows me to do some fairly nice changes to pipelines :)

russcam added a commit to elastic/elasticsearch-net that referenced this pull request Aug 4, 2020
russcam added a commit to elastic/elasticsearch-net that referenced this pull request Aug 4, 2020
github-actions bot pushed a commit to elastic/elasticsearch-net that referenced this pull request Aug 4, 2020
github-actions bot pushed a commit to elastic/elasticsearch-net that referenced this pull request Aug 4, 2020
russcam added a commit to elastic/elasticsearch-net that referenced this pull request Aug 5, 2020
Relates: elastic/elasticsearch#57030

Co-authored-by: Russ Cam <russ.cam@elastic.co>
russcam added a commit to elastic/elasticsearch-net that referenced this pull request Aug 5, 2020
Relates: elastic/elasticsearch#57030

Co-authored-by: Russ Cam <russ.cam@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >enhancement Team:Data Management Meta label for data/management team v7.9.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants