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
[FLINK-23487][FLINK-23635][s3] Update aws and presto-hive #16717
Conversation
Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community Automated ChecksLast check on commit 3911d13 (Sat Aug 28 13:10:20 UTC 2021) Warnings:
Mention the bot in a comment to re-run the automated checks. Review Progress
Please see the Pull Request Review Guide for a full explanation of the review process. The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required Bot commandsThe @flinkbot bot supports the following commands:
|
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 the quick fix @Airblader. I'm wondering whether we should wrap the original exception into a IOException
. It seems the file system usually does this for all kind of errors?
Newer versions are required to make IRSA (IAM Role for Service Account) work properly with S3. Since presto-hive now also directly depends on aws-java-sdk-sts, we make sure to exclude it from presto-hive and use our own version instead. This fixes IRSA for s3-presto, though not yet s3-hadoop. This closes apache#16592.
71cc3de
to
d602787
Compare
We previously expected and asserted on an IOException. However, presto-hive now uses the AWSCredentialsProviderChain when building the S3 client, and we instead fail with an SdkClientException.
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.
After an offline discussion with @AHeise we will let the SdkClientException
propagate as IOException
is more intended for retryability which is not the case for config issues.
Newer versions are required to make IRSA (IAM Role for Service Account) work properly with S3. Since presto-hive now also directly depends on aws-java-sdk-sts, we make sure to exclude it from presto-hive and use our own version instead. This fixes IRSA for s3-presto, though not yet s3-hadoop. This closes apache#16717.
See #16592 for description of the main PR. This PR is a fixup after it has been reverted.
We previously expected and asserted on an
IOException
. However, presto-hive now uses theAWSCredentialsProviderChain
when building the S3 client, and we instead fail with anSdkClientException
.See
Edit 1: The PR originally passed the CI because this test is not executed for PRs.
Edit 2: Since the original PR has been reverted in the meantime I have re-added that commit to this hotfix PR and updated the description.
Verifying this change
This change is already covered by existing tests.
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: noDocumentation