-
Notifications
You must be signed in to change notification settings - Fork 211
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 parse_ion processor #3803
Add parse_ion processor #3803
Conversation
Signed-off-by: Emma Becar <[email protected]>
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 this contribution!
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 @emmachase. I have a few comments to address. Other than that, we'll want to be sure it builds. Then we should be good to merge it in.
...src/main/java/org/opensearch/dataprepper/plugins/processor/parse/AbstractParseProcessor.java
Outdated
Show resolved
Hide resolved
...java/org/opensearch/dataprepper/plugins/processor/parse/ion/IonTimestampConverterModule.java
Show resolved
Hide resolved
...java/org/opensearch/dataprepper/plugins/processor/parse/ion/IonTimestampConverterModule.java
Show resolved
Hide resolved
...ssor/src/main/java/org/opensearch/dataprepper/plugins/processor/parse/CommonParseConfig.java
Show resolved
Hide resolved
Signed-off-by: Emma Becar <[email protected]>
Signed-off-by: Emma Becar <[email protected]>
@dlvenable Thanks for the review, the latest commits should resolve your comments :) |
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.
This looks good. Thank you @emmachase for this contribution!
Description
Add parse_ion processor.
This collects most of the logic based on the common config options into an abstract class AbstractParseProcessor.
Since Ion is a superset of JSON, I inherited the JSON test suite for the Ion processor, and added an additional test to ensure that the Ion specific features work correctly.
Note: Since eventually the parsed objects need to be converted to JSON, Ion Timestamps are parsed into ISO-8601 Z strings. Ion Blobs are parsed into base64 encoded strings as per default Jackson behavior. Annotations are currently stripped entirely; in the future we could possibly add config options to support exposing them as extra fields (e.g.
{foo: bar::"baz"}
=>{"foo": "baz", "foo$annotations": ["bar"]}
).Documentation Issue: opensearch-project/documentation-website#5769
Issues Resolved
Resolves #3730
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.