fs2-aws icon indicating copy to clipboard operation
fs2-aws copied to clipboard

S3 upload should make it easy to set properties on upload requests

Open blissd opened this issue 4 years ago • 9 comments

Problem

We have an fs2 job that runs in one AWS account and must upload files to a separate AWS account. To make sure the ownership/permissions of the files are correct we must set bucket owner full control on the upload request. However, the fs2-aws API makes this tricky to do.

Work Around

To set this property we extend the AbstractAmazonS23 class and implement just enough of the API to support uploading through fs2-aws. The code looks something like this:

object S3ClientWithCannedAcl extends AbstractAmazonS3 {

  private val client = AmazonS3ClientBuilder.defaultClient()

  override def initiateMultipartUpload(request: InitiateMultipartUploadRequest): InitiateMultipartUploadResult = {
    request.setCannedACL(CannedAccessControlList.BucketOwnerFullControl)
    client.initiateMultipartUpload(request)
  }

  override def uploadPart(request: UploadPartRequest): UploadPartResult = {
    client.uploadPart(request)
  }

 override def completeMultipartUpload(request: CompleteMultipartUploadRequest): CompleteMultipartUploadResult = {
    client.completeMultipartUpload(request)
  }
}

fs2.aws.s3.uploadS3FileMultipart[IO](bucket, key, chunkSize, amazonS3 = S3ClientWithCannedAcl)

It would be great if the fs2-aws API allowed for setting properties like this in a more direct way.

blissd avatar Jul 15 '20 08:07 blissd

This could be improved either by adding f: PutObjectRequest.Builder => PutObjectRequest.Builder parameter (with identity as a default) that would allow the user to modify the request. Alternatively we could add each relevant parameter to the signature (in the example above: metadata, contentType, etc.)

If someone could point me to the preferred alternative I could work on this and open a PR.

fredshonorio avatar Feb 27 '21 18:02 fredshonorio

I think things changed since this Issue was create, we migrated to SDK V2. But the problem still here https://github.com/laserdisc-io/fs2-aws/blob/master/fs2-aws-s3/src/main/scala/fs2/aws/s3.scala#L115 I see the necessity of better control of the AWS SDK requests parameters. Suggestions are welcome.

semenodm avatar Mar 01 '21 14:03 semenodm

@semenodm What do you mean by "better control" here? Is it about not exposing the builder?

fredshonorio avatar Mar 01 '21 14:03 fredshonorio

well, we might want to expose the Requests contractions to the user (developer), i'm just thinking of better way doing this. In this particular case we have to expose several requests: CreateMultipartUploadRequest UploadPartRequest CompleteMultipartUploadRequest AbortMultipartUploadRequest so there are 4 of them, i don't like the idea to provide 4 functions to do this, instead we might want to provide some Algebra with identity default behavior

semenodm avatar Mar 01 '21 15:03 semenodm

I see, I did not consider those "internal" requests. What would that algebra look like?

fredshonorio avatar Mar 01 '21 15:03 fredshonorio

well, i'm thinking of having the trait with 4 methods to override the request builder like you mentioned above. the default implementation would be identity for every builder again as you stated earlier. so every time we need to call this overrider for every internal requests construction. At this point i don't know if we can do better. also i would like to keep existing API, just use default overrider for it

semenodm avatar Mar 02 '21 15:03 semenodm

Ok, I can take a stab at this soon

fredshonorio avatar Mar 02 '21 15:03 fredshonorio

Thank you @fredshonorio for help

semenodm avatar Mar 02 '21 16:03 semenodm

Added #740 to address this.

fredshonorio avatar Sep 15 '21 18:09 fredshonorio

Closing it, take a look at request interceptors in latest algebra

semenodm avatar Dec 01 '23 17:12 semenodm