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

Improved Avro error reporting related to schemas #3110

Merged
merged 2 commits into from
Aug 10, 2023

Conversation

dlvenable
Copy link
Member

Description

This PR has two major updates:

  • When starting Data Prepper, validate the provided schema. Prior to this PR it was happening only when an event is received.
  • When a field does not exist in the Avro schema return an explicit exception. Prior to this PR, it was some vague message that was not possible to track down.

Additionally, this PR includes some code clean-up.

Issues Resolved

N/A

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.

cmanning09
cmanning09 previously approved these changes Aug 8, 2023
Copy link
Contributor

@cmanning09 cmanning09 left a comment

Choose a reason for hiding this comment

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

LG - The only feedback I have are stylistic based on the code clean up you have already done.

@@ -179,6 +176,9 @@ private GenericRecord buildAvroRecord(final Schema schema, final Map<String, Obj
continue;
}
final Schema.Field field = schema.getField(key);
if(field == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

(Only because you fixed other spacing issues in this PR)

You are missing a space after the if

if (field == null) {

Comment on lines +47 to +48
final Map<String, Object> stringObjectMap = objectMapper.readValue(s3Object, new TypeReference<>() {
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this change intentional to split into two lines?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is what my IntelliJ formatter produced.

…exception when an Avro field is missing from the schema.

Signed-off-by: David Venable <dlv@amazon.com>
Signed-off-by: David Venable <dlv@amazon.com>
@dlvenable
Copy link
Member Author

@cmanning09 , @chenqi0805

I had to rebase and force push due to changes from #3122. Please take another look. Thanks!

@dlvenable dlvenable merged commit 244524b into opensearch-project:main Aug 10, 2023
24 checks passed
@dlvenable dlvenable deleted the avro-schema-startup branch November 10, 2023 19:24
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.

3 participants