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

Source Codecs | Avro Codec follow-on PR #2715

Merged
merged 5 commits into from
Jun 3, 2023

Conversation

umayr-codes
Copy link
Contributor

Description

Follow on PR on Avro codecs with support for nested and other non-primitive data types.

Issues Resolved

Resolves #1532

Check List

  • New functionality includes testing.
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

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.

@umayr-codes umayr-codes changed the title Sink codecs Source Codecs | Avro Codec follow-on PR May 19, 2023
@umayr-codes umayr-codes mentioned this pull request May 19, 2023
4 tasks
return event;
}

private static Map<String, Object> convertRecordToMap(GenericRecord record, Schema schema) throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

You are duplicating the logic from AvroInputCodec in the test. This is not a valid test scenario. You should start with known data coming in and verify it against expected data going out.

Please remove this entire function - convertRecordToMap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is done @dlvenable !

Signed-off-by: umairofficial <[email protected]>
@codecov
Copy link

codecov bot commented May 26, 2023

Codecov Report

Merging #2715 (09df7ed) into main (d64b48c) will increase coverage by 0.66%.
The diff coverage is n/a.

❗ Current head 09df7ed differs from pull request most recent head 3956d39. Consider uploading reports for the commit 3956d39 to get more accurate results

@@             Coverage Diff              @@
##               main    #2715      +/-   ##
============================================
+ Coverage     93.45%   94.12%   +0.66%     
- Complexity     2264     2440     +176     
============================================
  Files           263      278      +15     
  Lines          6330     6744     +414     
  Branches        526      550      +24     
============================================
+ Hits           5916     6348     +432     
+ Misses          277      256      -21     
- Partials        137      140       +3     

see 47 files with indirect coverage changes

Signed-off-by: umairofficial <[email protected]>
@dlvenable
Copy link
Member

@umayr-codes , Did you mean to include work for the output codecs in this PR?

Signed-off-by: umairofficial <[email protected]>
@umayr-codes
Copy link
Contributor Author

@umayr-codes , Did you mean to include work for the output codecs in this PR?

No. That commit has been rolled back.

@kkondaka kkondaka merged commit affe0b2 into opensearch-project:main Jun 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support generic parsers/codecs
4 participants