ahmarsuhail commented on code in PR #5421:
URL: https://github.com/apache/hadoop/pull/5421#discussion_r1142313258
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:
##########
@@ -834,58 +834,28 @@ protected static S3AStorageStatistics
createStorageStatistics(
* @throws UnknownStoreException the bucket is absent
* @throws IOException any other problem talking to S3
*/
- // TODO: Review: this used to call doesBucketExist in v1, which does not
check permissions,
- // not even read access.
@Retries.RetryTranslated
protected void verifyBucketExists() throws UnknownStoreException,
IOException {
if (!invoker.retry("doesBucketExist", bucket, true,
trackDurationOfOperation(getDurationTrackerFactory(),
STORE_EXISTS_PROBE.getSymbol(),
() -> {
try {
+ if (bucketRegions.containsKey(bucket)) {
+ return true;
+ }
s3Client.headBucket(HeadBucketRequest.builder().bucket(bucket).build());
return true;
- } catch (NoSuchBucketException e) {
- return false;
- }
- }))) {
- throw new UnknownStoreException("s3a://" + bucket + "/", " Bucket does "
+ "not exist");
- }
- }
-
- /**
- * Verify that the bucket exists. This will correctly throw an exception
- * when credentials are invalid.
- * TODO: Review. May be redundant in v2.
- * Retry policy: retrying, translated.
- * @throws UnknownStoreException the bucket is absent
- * @throws IOException any other problem talking to S3
- */
- @Retries.RetryTranslated
- protected void verifyBucketExistsV2()
- throws UnknownStoreException, IOException {
- if (!invoker.retry("doesBucketExistV2", bucket, true,
- trackDurationOfOperation(getDurationTrackerFactory(),
- STORE_EXISTS_PROBE.getSymbol(),
- () -> {
- // Bug in SDK always returns `true` for AccessPoint ARNs with
`doesBucketExistV2()`
- // expanding implementation to use ARNs and buckets correctly
- try {
- s3Client.getBucketAcl(GetBucketAclRequest.builder()
- .bucket(bucket)
- .build());
} catch (AwsServiceException ex) {
int statusCode = ex.statusCode();
if (statusCode == SC_404_NOT_FOUND ||
- (statusCode == SC_403_FORBIDDEN &&
- ex.getMessage().contains(AP_INACCESSIBLE))) {
+ (statusCode == SC_403_FORBIDDEN && accessPoint != null)) {
return false;
}
}
return true;
}))) {
- throw new UnknownStoreException("s3a://" + bucket + "/", " Bucket does "
- + "not exist");
+ throw new UnknownStoreException("s3a://" + bucket + "/", " Bucket does "
+ "not exist");
Review Comment:
have included the endpoint. but wondering, in the `getS3Region()` probe, we
build a `s3Client` with the region `eu-west-2` to do a region probe. This
client does not have an endpoint configured, so this probe will fail for
on-prem and third party stores. is there anything we need to do for this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
