-
Notifications
You must be signed in to change notification settings - Fork 244
Added array support for ndjson inputcodec #5875
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
Added array support for ndjson inputcodec #5875
Conversation
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.
Thanks for providing this change. Looks good to me.
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.
Thank you @sivagnanam-arumugam for this contribution!
The DCO check is failing: https://github.com/opensearch-project/data-prepper/pull/5875/checks?check_run_id=45354977732
void parse_with_array_of_objects_asserts_number_of_objects() throws IOException { | ||
final NdjsonInputCodec objectUnderTest = createObjectUnderTest(); | ||
|
||
final String jsonArray = "[{\"key1\":\"value1\"},{\"key2\":\"value2\"}]"; |
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.
Does this change support reading multiple arrays from different lines?
[{\"key1\":\"value1\"},{\"key2\":\"value2\"}]
[{\"key3\":\"value3\"},{\"key4\":\"value4\"}]
If so, we should test this too.
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.
Added multiple array support and added unit test cases.
Added signed-off-by commit message.
Signed-off-by: sivagnanam-arumugam <[email protected]>
Added test cases for multiple array support Signed-off-by: sivagnanam-arumugam <[email protected]>
27c9f6e
to
366f512
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.
Thank you @sivagnanam-arumugam for this contribution and follow up changes!
@KarstenSchnitter , Would you like to take another look?
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.
Still looks good to me. Thanks for the contribution.
Description
Array support for ndjson input codec,
Array line is handled in logstash json_lines codec, So added the support to data prepper.
Issues Resolved
Resolves #5874
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.