Skip to content

HADOOP-18073. Address review comments. #31

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@

package org.apache.hadoop.fs.s3a;

import software.amazon.awssdk.core.exception.SdkException;
import org.apache.hadoop.util.Preconditions;

import java.io.IOException;

import software.amazon.awssdk.core.exception.SdkException;

/**
* IOException equivalent of an {@link SdkException}.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@

import com.amazonaws.auth.AWSCredentials;
import com.amazonaws.auth.AWSCredentialsProvider;
import com.amazonaws.auth.BasicAWSCredentials;
import com.amazonaws.auth.BasicSessionCredentials;
import org.apache.hadoop.classification.VisibleForTesting;
import org.apache.hadoop.fs.s3a.adapter.V1V2AwsCredentialProviderAdapter;
import org.apache.hadoop.util.Preconditions;

import com.amazonaws.auth.BasicAWSCredentials;
import com.amazonaws.auth.BasicSessionCredentials;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@

package org.apache.hadoop.fs.s3a;

import software.amazon.awssdk.awscore.exception.AwsServiceException;
import software.amazon.awssdk.awscore.exception.AwsErrorDetails;
import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;

import software.amazon.awssdk.awscore.exception.AwsErrorDetails;
import software.amazon.awssdk.awscore.exception.AwsServiceException;

/**
* A specific exception from AWS operations.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,7 @@ public CredentialInitializationException(String message) {
* @return false, always.
*/
@Override
public boolean retryable() { return false; }
public boolean retryable() {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import com.amazonaws.util.AwsHostNameUtils;
import com.amazonaws.util.RuntimeHttpUtils;

import org.apache.hadoop.fs.s3a.impl.AWSClientConfig;
import org.apache.hadoop.util.Preconditions;
import org.apache.hadoop.classification.VisibleForTesting;
import org.slf4j.Logger;
Expand Down Expand Up @@ -72,7 +73,7 @@
import org.apache.hadoop.fs.s3a.statistics.impl.AwsStatisticsCollector;
import org.apache.hadoop.fs.store.LogExactlyOnce;

import static com.amazonaws.services.s3.Headers.REQUESTER_PAYS_HEADER;
import static org.apache.hadoop.fs.s3a.impl.AWSHeaders.REQUESTER_PAYS_HEADER;
import static org.apache.hadoop.fs.s3a.Constants.AWS_REGION;
import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_CENTRAL_REGION;
import static org.apache.hadoop.fs.s3a.Constants.BUCKET_REGION_HEADER;
Expand Down Expand Up @@ -227,25 +228,20 @@ public S3AsyncClient createS3AsyncClient(
* @param <BuilderT> S3 client builder type
* @param <ClientT> S3 client type
*/
private <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT>
BuilderT configureClientBuilder(
BuilderT builder,
S3ClientCreationParameters parameters,
Configuration conf,
String bucket) {
private <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT> BuilderT configureClientBuilder(
BuilderT builder, S3ClientCreationParameters parameters, Configuration conf, String bucket) {

URI endpoint = getS3Endpoint(parameters.getEndpoint(), conf);
Region region = getS3Region(conf.getTrimmed(AWS_REGION), bucket,
parameters.getCredentialSet());
Region region = getS3Region(conf.getTrimmed(AWS_REGION), bucket, parameters.getCredentialSet());
LOG.debug("Using endpoint {}; and region {}", endpoint, region);

// TODO: Some configuration done in configureBasicParams is not done yet.
S3Configuration serviceConfiguration = S3Configuration.builder()
.pathStyleAccessEnabled(parameters.isPathStyleAccess())
// TODO: Review. Currently required to pass access point tests in ITestS3ABucketExistence,
// but resolving the region from the ap may be the correct solution.
.useArnRegionEnabled(true)
.build();
.pathStyleAccessEnabled(parameters.isPathStyleAccess())
// TODO: Review. Currently required to pass access point tests in ITestS3ABucketExistence,
// but resolving the region from the ap may be the correct solution.
.useArnRegionEnabled(true)
.build();

return builder
.overrideConfiguration(createClientOverrideConfiguration(parameters, conf))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ private static class FailureInjectionInterceptor implements ExecutionInterceptor
*/
private final AtomicLong failureCounter = new AtomicLong(0);

public FailureInjectionInterceptor(FailureInjectionPolicy policy) {
FailureInjectionInterceptor(FailureInjectionPolicy policy) {
this.policy = policy;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.concurrent.Future;
import javax.annotation.Nullable;

import software.amazon.awssdk.core.exception.SdkException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -37,7 +38,6 @@
import org.apache.hadoop.util.functional.InvocationRaisingIOE;
import org.apache.hadoop.util.Preconditions;

import software.amazon.awssdk.core.exception.SdkException;

import static org.apache.hadoop.fs.statistics.impl.IOStatisticsBinding.invokeTrackingDuration;

Expand Down Expand Up @@ -444,7 +444,7 @@ public <T> T retryUntranslated(
* @param operation operation to execute
* @return the result of the call
* @throws IOException any IOE raised
* @throws SdkBaseException any AWS exception raised
* @throws SdkException any AWS exception raised
* @throws RuntimeException : these are never caught and retries.
*/
@Retries.RetryRaw
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

package org.apache.hadoop.fs.s3a;

import software.amazon.awssdk.services.s3.model.CommonPrefix;
import software.amazon.awssdk.services.s3.model.S3Object;

import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.VisibleForTesting;
Expand All @@ -38,8 +40,6 @@
import org.apache.hadoop.util.functional.RemoteIterators;

import org.slf4j.Logger;
import software.amazon.awssdk.services.s3.model.CommonPrefix;
import software.amazon.awssdk.services.s3.model.S3Object;

import java.io.Closeable;
import java.io.IOException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,10 @@
import java.nio.file.AccessDeniedException;
import java.util.List;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import software.amazon.awssdk.services.s3.model.S3Error;
import software.amazon.awssdk.services.s3.model.S3Exception;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
Expand Down Expand Up @@ -61,7 +60,9 @@ public MultiObjectDeleteException(List<S3Error> errors) {
this.errors = errors;
}

public List<S3Error> errors() { return errors; }
public List<S3Error> errors() {
return errors;
}

/**
* A {@code MultiObjectDeleteException} is raised if one or more
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@
import java.util.NoSuchElementException;
import javax.annotation.Nullable;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import software.amazon.awssdk.services.s3.S3Client;
import software.amazon.awssdk.services.s3.model.ListMultipartUploadsRequest;
import software.amazon.awssdk.services.s3.model.ListMultipartUploadsResponse;
import software.amazon.awssdk.services.s3.model.MultipartUpload;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;


import org.apache.hadoop.fs.RemoteIterator;
import org.apache.hadoop.fs.s3a.api.RequestFactory;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,11 @@

package org.apache.hadoop.fs.s3a;

import org.slf4j.Logger;

import software.amazon.awssdk.transfer.s3.ObjectTransfer;
import software.amazon.awssdk.transfer.s3.progress.TransferListener;

import org.apache.hadoop.util.Progressable;
import org.slf4j.Logger;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this supposed to go in the same block as software..?



/**
* Listener to progress from AWS regarding transfers.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;

import software.amazon.awssdk.core.exception.SdkException;
import software.amazon.awssdk.core.sync.RequestBody;
import software.amazon.awssdk.services.s3.model.CompletedPart;
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
import software.amazon.awssdk.services.s3.model.PutObjectResponse;
import software.amazon.awssdk.services.s3.model.UploadPartRequest;
import software.amazon.awssdk.services.s3.model.UploadPartResponse;
import com.amazonaws.event.ProgressEvent;
import com.amazonaws.event.ProgressEventType;
import com.amazonaws.event.ProgressListener;
Expand All @@ -45,14 +52,6 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import software.amazon.awssdk.core.exception.SdkException;
import software.amazon.awssdk.core.sync.RequestBody;
import software.amazon.awssdk.services.s3.model.CompletedPart;
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
import software.amazon.awssdk.services.s3.model.PutObjectResponse;
import software.amazon.awssdk.services.s3.model.UploadPartRequest;
import software.amazon.awssdk.services.s3.model.UploadPartResponse;

import org.apache.hadoop.classification.InterfaceAudience;
import org.apache.hadoop.classification.InterfaceStability;
import org.apache.hadoop.fs.Abortable;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,15 @@ static BlockFactory createFactory(S3AFileSystem owner,
* It can be one of a file or an input stream.
* When closed, any stream is closed. Any source file is untouched.
*/
public static final class BlockUploadData implements Closeable {
public static final class BlockUploadData implements Closeable {
private final File file;
private final InputStream uploadStream;

/**
* File constructor; input stream will be null.
* @param file file to upload
*/
public BlockUploadData(File file) {
public BlockUploadData(File file) {
Preconditions.checkArgument(file.exists(), "No file: " + file);
this.file = file;
this.uploadStream = null;
Expand All @@ -119,7 +119,7 @@ public BlockUploadData(File file) {
* Stream constructor, file field will be null.
* @param uploadStream stream to upload
*/
public BlockUploadData(InputStream uploadStream) {
public BlockUploadData(InputStream uploadStream) {
Preconditions.checkNotNull(uploadStream, "rawUploadStream");
this.uploadStream = uploadStream;
this.file = null;
Expand Down
Loading