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

[SPARK-42057][SQL][PROTOBUF] Fix how exception is handled in error reporting. #39536

Closed
wants to merge 4 commits into from

Conversation

rangadi
Copy link
Contributor

@rangadi rangadi commented Jan 12, 2023

What changes were proposed in this pull request?

Protobuf connector related error handlers incorrectly report the exception. This is makes it hard for users to see actual issue. E.g. if there is a FileNotFoundException these error handlers use pass exception.getCause() rather than passing exception. As result, we lose the information that it was a FileNotFoundException

This PR fixes that.

Why are the changes needed?

This improves error reporting.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

  • A new test is added.

@rangadi
Copy link
Contributor Author

rangadi commented Jan 12, 2023

@SandishKumarHN please take a look.

@@ -229,7 +229,7 @@ private[sql] object ProtobufUtils extends Logging {
fileDescriptorSet = DescriptorProtos.FileDescriptorSet.parseFrom(dscFile)
} catch {
case ex: InvalidProtocolBufferException =>
throw QueryCompilationErrors.descrioptorParseError(descFilePath, ex)
throw QueryCompilationErrors.descriptorParseError(descFilePath, ex)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a typo fix in function name.

@rangadi rangadi changed the title [SQL][PROTOBUF] Fix how exception is handled in error reporting. [SPARK-42057][SQL][PROTOBUF] Fix how exception is handled in error reporting. Jan 13, 2023
Copy link
Contributor

@SandishKumarHN SandishKumarHN left a comment

Choose a reason for hiding this comment

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

this makes sense, not sure how we missed it.

LGTM.

@rangadi rangadi requested review from gengliangwang and removed request for gengliangwang January 13, 2023 20:00
@gengliangwang
Copy link
Member

Thanks, merging to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants