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 empty_value parameter to CSV processor #51567

Merged
merged 6 commits into from
Feb 5, 2020

Conversation

probakowski
Copy link
Contributor

This change adds empty_value parameter to the CSV processor.
This value is used to fill empty fields. Fields will be skipped
if this parameter is omitted. This behavior is the same for both
quoted and unquoted fields.

This change adds `empty_value` parameter to the CSV processor.
This value is used to fill empty fields. Fields will be skipped
if this parameter is ommited. This behavior is the same for both
quoted and unquoted fields.
@probakowski probakowski added >enhancement :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP v8.0.0 v7.7.0 labels Jan 28, 2020
@elasticmachine
Copy link
Collaborator

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

Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

I also tested it in conjunction with #51492 and elastic/kibana#56038 and confirmed that the end-to-end import process now works with CSV files that have quoted empty fields and also those that have unquoted empty fields. 🎉

@@ -158,7 +218,7 @@ public void testEmptyFields() throws Exception {
items.keySet().stream().skip(numItems - 1).forEach(key -> assertFalse(ingestDocument.hasField(key)));
}

public void testWrongStings() throws Exception {
public void testWrongStings() {
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: Sting -> String

(But it was there before and you've got a green build so if no other changes are required then no need to trigger a rebuild just for this.)

Copy link
Member

@jbaiera jbaiera left a comment

Choose a reason for hiding this comment

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

One quick suggestion for the docs, but otherwise LGTM

docs/reference/ingest/processors/csv.asciidoc Outdated Show resolved Hide resolved
@probakowski
Copy link
Contributor Author

@elasticmachine update branch

@probakowski
Copy link
Contributor Author

@elasticmachine update branch

@probakowski probakowski merged commit 5560135 into elastic:master Feb 5, 2020
@probakowski probakowski deleted the csv-processor-emptyvalue branch February 5, 2020 21:36
probakowski added a commit that referenced this pull request Feb 5, 2020
* Add empty_value parameter to CSV processor

This change adds `empty_value` parameter to the CSV processor.
This value is used to fill empty fields. Fields will be skipped
if this parameter is ommited. This behavior is the same for both
quoted and unquoted fields.

* docs updated

* Fix compilation problem

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
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 v7.7.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants