-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
ARROW-17079: [C++] Raise proper error message instead of error code for S3 errors #14001
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 @pcmoritz , let's wait for CI now.
Sounds great! I hope the inline won't lead to code explosion -- do you think that's a concern? |
Well, "inline" is a bit a of a misnomer nowadays and it doesn't force inlining. |
Hmm... looks like you need to be a bit more careful about which enum values exist: |
Thanks, I updated the PR and left a comment about it (one of the builds is using an older version of the SDK that doesn't have this error in the enum yet). This error was added ~2 years ago, all others have been there for >= 4 years, so we should be good now :) I'm planning to merge the PR if the CI passes. |
Benchmark runs are scheduled for baseline = 13a7b60 and contender = 46f38dc. 46f38dc is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
['Python', 'R'] benchmarks have high level of regressions. |
…or S3 errors (apache#14001) Part 2 to bring our S3 error messages up to the same standard as the ones from boto3. The error types are from https://github.com/aws/aws-sdk-cpp/blob/main/aws-cpp-sdk-s3/include/aws/s3/S3Errors.h#L16 -- unfortunately the AWS C++ doesn't seem to have a way to get the errors programmatically from the error codes so we had to hand code them. The new error format is: > When getting information for key 'test.csv' in bucket 'pcmoritz-test-bucket-arrow-errors': AWS Error **ACCESS_DENIED** during HeadObject operation: No response body. The old format was: > When getting information for key 'test.csv' in bucket 'pcmoritz-test-bucket-arrow-errors': AWS Error **[code 15]** during HeadObject operation: No response body. Authored-by: Philipp Moritz <[email protected]> Signed-off-by: Philipp Moritz <[email protected]>
…or S3 errors (apache#14001) Part 2 to bring our S3 error messages up to the same standard as the ones from boto3. The error types are from https://github.com/aws/aws-sdk-cpp/blob/main/aws-cpp-sdk-s3/include/aws/s3/S3Errors.h#L16 -- unfortunately the AWS C++ doesn't seem to have a way to get the errors programmatically from the error codes so we had to hand code them. The new error format is: > When getting information for key 'test.csv' in bucket 'pcmoritz-test-bucket-arrow-errors': AWS Error **ACCESS_DENIED** during HeadObject operation: No response body. The old format was: > When getting information for key 'test.csv' in bucket 'pcmoritz-test-bucket-arrow-errors': AWS Error **[code 15]** during HeadObject operation: No response body. Authored-by: Philipp Moritz <[email protected]> Signed-off-by: Philipp Moritz <[email protected]>
Part 2 to bring our S3 error messages up to the same standard as the ones from boto3.
The error types are from https://github.com/aws/aws-sdk-cpp/blob/main/aws-cpp-sdk-s3/include/aws/s3/S3Errors.h#L16 -- unfortunately the AWS C++ doesn't seem to have a way to get the errors programmatically from the error codes so we had to hand code them.
The new error format is:
The old format was: