laserdisc-io/fs2-aws

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

blissd opened this issue · 10 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.

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.

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 What do you mean by "better control" here? Is it about not exposing the builder?

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

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

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

Ok, I can take a stab at this soon

Thank you @fredshonorio for help

Added #740 to address this.

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