Adds tests for presigned operations and some improvements #96
Conversation
| @@ -517,3 +761,6 @@ def copyObjectTest(s3, source_bucket_name, target_bucket_name, data_dir, source_ | |||
| aws.getObjectTest(s3Resource, bucket_name1, File.join(data_dir, file_name1), destination) | |||
| aws.copyObjectTest(s3Resource, bucket_name1, bucket_name2, data_dir, file_name1) | |||
| aws.copyObjectTest(s3Resource, bucket_name1, bucket_name2, data_dir, file_name1, file_new_name) | |||
| aws.presignedGetObjectTest(s3Resource, bucket_name1, data_dir, file_name1) | |||
There was a problem hiding this comment.
Passing the client object in seems very non standard. Can we make s3Resource a member of the class?
| # of the progress message | ||
| msg = "\t" + log_msg + "%s" + "\n" | ||
| printf(msg.light_black, arg) | ||
| end |
There was a problem hiding this comment.
@ebozduman , it might be a good idea to use ruby Logger class for logging instead of print_* methods.
| removeObjects(s3, b) | ||
| removeBucket(s3, b) | ||
| end | ||
| end | ||
| print_logn("- Success!") |
There was a problem hiding this comment.
Instead of interspersing print statements through the code, it would be better to log minimally - once on successful cleanup or log error on failure. Also, it might help to log the function name, line # and specific error message
| begin | ||
| s3 = create_s3_resource | ||
| print_log("Making bucket: ", bucket_name) | ||
| makeBucket(s3, bucket_name) |
There was a problem hiding this comment.
Moved this deeper in the code so that the calling function doesnot have to pass s3. By making it a member variable of the class awssdkruby, all methods can operate on this without having to pass it around. Since almost every method needs it, its a natural fit for being a class variable.
This is fine with me to address later instead of in this pr.
|
@poornas anything pending here ? |
|
I understand #102 was filed for log refactor. |
* Creates new AWS Ruby SDK api commands and tests
Issue #93