Cache the s3 client

[jhorbulyk]

Would you consider modifying the code so that if the user does not provide their own s3 client, then Uploader will create a client once and cache that client object so that it can be used for multiple Uploader() calls?

If anyone is using this library to make multiple uploads to the same s3 account, then they wouldn't want to create the s3 object multiple times. And it would seem cumbersome to create a wrapper around this wrapper that creates a cached s3Client object.

[jhorbulyk]

If this is something that you would consider incorporating, I can also write the code and make a pull request.

[nathanpeck]

I understand the desire to not create a new client instance each time that you upload, but what is preventing the following snippet from being an acceptable pattern?

var AWS = require('aws-sdk'),
      Uploader = require('s3-upload-stream').Uploader,
      yourCachedS3Client = new AWS.S3();

function usingTheUploadStream() {
   var thisUpload = new Uploader(
      { s3Client: yourCachedS3Client },
      {
         Bucket: "your-bucket-name",
         Key: "uploaded-file-name " + new Date()
      },
      function (err, uploadStream) {
         /*...use the upload stream here...*/
      }
   );
}

I feel this is a fairly lightweight way to achieve the caching that you are requesting.

However, I do agree that an interface like this for caching the client could be a bit nicer:

var AWS = require('aws-sdk'),
      s3UploadStream = require('s3-upload-stream'),
      Uploader = s3UploadStream.Uploader,
      yourCachedS3Client = new AWS.S3();

s3UploadStream.setClient(yourCachedS3Client);

function usingTheUploadStream() {
   var thisUpload = new Uploader(
      {
         Bucket: "your-bucket-name",
         Key: "uploaded-file-name " + new Date()
      },
      function (err, uploadStream) {
         /*...use the upload stream here...*/
      }
   );
}

If you would like to develop the setClient style method so that you only have to set the client once I'm open to merging that in. Just make sure that it doesn't break support for the legacy users who are passing a client in with each request, or passing in credentials.

[jhorbulyk]

The first snippet would be fine, apart from the fact that it is a wrapper around a wrapper offering functionality that many people would want in the original wrapper. I'll develop a setClient style method and submit a pull request tomorrow.

[nathanpeck]

Sure, that's fine with me. Thanks!

[nathanpeck]

I've added caching to the module in version 0.5.0

Let me know if this works for you.