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

S3CrtAsyncClientBuilder.build() should return S3CrtAsyncClient #5057

Open
brianwebb11 opened this issue Mar 30, 2024 · 3 comments
Open

S3CrtAsyncClientBuilder.build() should return S3CrtAsyncClient #5057

brianwebb11 opened this issue Mar 30, 2024 · 3 comments
Assignees
Labels
feature-request A feature should be added or improved.

Comments

@brianwebb11
Copy link

Describe the bug

I am unable to build an instance of type S3CrtAsyncClient

The document has the following example:
S3AsyncClient s3AsyncClient = S3AsyncClient.crtCreate();

This will indeed build an instance of S3CrtAsyncClient, however the return type of crtCreate() is S3AsyncClient, not S3CrtAsyncClient.

So I can't have code that requires S3CrtAsyncClient without manually casting.

Expected Behavior

I was hoping to have the signature be

S3CrtAsyncClient S3CrtAsyncClientBuilder.build();
instead of the current code
S3AsyncClient S3CrtAsyncClientBuilder.build();

Current Behavior

S3CrtAsyncClient S3CrtAsyncClientBuilder.build();
See https://github.com/aws/aws-sdk-java-v2/blob/master/services/s3/src/main/java/software/amazon/awssdk/services/s3/S3CrtAsyncClientBuilder.java#L339

Reproduction Steps

S3CrtAsyncClient s3AsyncClient = S3AsyncClient.crtCreate();

fails to compile.

Possible Solution

Change
S3AsyncClient S3CrtAsyncClientBuilder.build();
to
S3CrtAsyncClient S3CrtAsyncClientBuilder.build();
here https://github.com/aws/aws-sdk-java-v2/blob/master/services/s3/src/main/java/software/amazon/awssdk/services/s3/S3CrtAsyncClientBuilder.java#L339

Additional Information/Context

No response

AWS Java SDK version used

2.23.4

JDK version used

openjdk 11.0.21 2023-10-17 OpenJDK Runtime Environment Homebrew (build 11.0.21+0) OpenJDK 64-Bit Server VM Homebrew (build 11.0.21+0, mixed mode)

Operating System and version

MacOS

@brianwebb11 brianwebb11 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 30, 2024
@debora-ito
Copy link
Member

So I can't have code that requires S3CrtAsyncClient without manually casting.

@brwebb can you give us more context about your use case? How would you use the S3CrtAsyncClient and why it's required?

@debora-ito debora-ito added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. feature-request A feature should be added or improved. and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 3, 2024
@debora-ito debora-ito self-assigned this Apr 3, 2024
@brianwebb11
Copy link
Author

brianwebb11 commented Apr 3, 2024

Sure. The use case is that I want to upload a java InputStream to S3. According to the AWS docs you need to either use the AWS CRT-based S3 Client or Amazon S3 Transfer Manager.

Let's assume we want to use the AWS CRT-based S3 Client. As a developer, I know that the normal S3AsyncClient will NOT work. I need to use S3CrtAsyncClient. Let's take the example code from the AWS docs:

import com.example.s3.util.AsyncExampleUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import software.amazon.awssdk.core.async.AsyncRequestBody;
import software.amazon.awssdk.core.async.BlockingInputStreamAsyncRequestBody;
import software.amazon.awssdk.core.exception.SdkException;
import software.amazon.awssdk.services.s3.S3AsyncClient;
import software.amazon.awssdk.services.s3.model.PutObjectResponse;

import java.io.ByteArrayInputStream;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;

    /**
     * @param s33CrtAsyncClient - To upload content from a stream of unknown size, use the AWS CRT-based S3 client. For more information, see
     *                          https://docs.aws.amazon.com/sdk-for-java/latest/developer-guide/crt-based-s3-client.html.
     * @param bucketName - The name of the bucket.
     * @param key - The name of the object.
     * @return software.amazon.awssdk.services.s3.model.PutObjectResponse - Returns metadata pertaining to the put object operation.
     */
    public PutObjectResponse putObjectFromStream(S3AsyncClient s33CrtAsyncClient, String bucketName, String key) {

        BlockingInputStreamAsyncRequestBody body =
                AsyncRequestBody.forBlockingInputStream(null); // 'null' indicates a stream will be provided later.

        CompletableFuture<PutObjectResponse> responseFuture =
                s33CrtAsyncClient.putObject(r -> r.bucket(bucketName).key(key), body);

        // AsyncExampleUtils.randomString() returns a random string up to 100 characters.
        String randomString = AsyncExampleUtils.randomString();
        logger.info("random string to upload: {}: length={}", randomString, randomString.length());

        // Provide the stream of data to be uploaded.
        body.writeInputStream(new ByteArrayInputStream(randomString.getBytes()));

        PutObjectResponse response = responseFuture.join(); // Wait for the response.
        logger.info("Object {} uploaded to bucket {}.", key, bucketName);
        return response;
    }
}

In that sample code, the method signature allows a caller is allowed to pass in an object of type S3AsyncClient. However, we know this will fail at runtime. The docs state that if you don't know the size of the InputStream then you need to use S3CrtAsyncClient. So as the implementer of putObjectFromStream(), it would be better to change the method signature to require the first parameter to be S3CrtAsyncClient. This forces callers to pass arguments that can work and prevents callers from passing arguments that will never work.

If the method signature requires S3CrtAsyncClient, then the caller needs to be able to build an instance of S3CrtAsyncClient. According to the AWS docs, the recommended way to build this object is S3AsyncClient.crtCreate();. But this method returns a S3AsyncClient when we need a S3CrtAsyncClient.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 10 days. label Apr 4, 2024
@hdeweirdt
Copy link

hdeweirdt commented May 2, 2024

To add to this: the current sample in the aws-sdk-java-v2 for async putobject does not even work without changing the code something similar to this:

PutObjectFromStreamAsync example = new PutObjectFromStreamAsync();
            S3CrtAsyncClient client = ((S3CrtAsyncClient) S3CrtAsyncClient.builder().credentialsProvider(DefaultCredentialsProvider.create()).build());
            PutObjectResponse putObjectResponse = example.putObjectFromStream(client, bucketName, key);
            logger.info("Object {} etag: {}", key, putObjectResponse.eTag());
            logger.info("Object {} uploaded to bucket {}.", key, bucketName);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

3 participants