-
Notifications
You must be signed in to change notification settings - Fork 733
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_failure' option to ingest processors #2003
Conversation
55790cc
to
80c19dd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change LGTM. One thing I wonder is why you updated so many tests? Is it required? Did the default change?
$processor = new SplitProcessor('joined_array_field', '-'); | ||
$processor->setIgnoreMissing(true); | ||
$processor = (new SplitProcessor('joined_array_field', '-')) | ||
->setIgnoreFailure(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming this test was not updated, I assume it would just have worked as before as the default did not change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testSplitWithNonDefaultOptions
I interpreted this test as testing options that are not put by default, so I added the new option here to test it was rightly set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, makes sense!
80c19dd
to
31474ec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
$processor = new SplitProcessor('joined_array_field', '-'); | ||
$processor->setIgnoreMissing(true); | ||
$processor = (new SplitProcessor('joined_array_field', '-')) | ||
->setIgnoreFailure(true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, makes sense!
No description provided.