Update minio-go #2299

harshavardhana · 2017-11-02T23:19:30Z

Avoid setting server encryption headers with x-amz-meta (Update to new changes in minio-go #859)
Remove redirectHeaders method (Add atomic package for atomic file writes, use that in quick config #857)
Honor file pointer set in PutObject method (Avoid possible corrupted data when saving with quick #856)
Fix docs and add a validator tool for our api docs (User struct{} instead of bool for saving session, saves a 1byte #850)
requestMetadata should take the md5 and sha256 with expected encodings. (Prettify the progress bar output #852)
Added a new method SetUserMetadata to the PostPolicy (Rename isMcConfigExist to isMcConfigExists for brevity #839)
Do not need to strictly validate request params in Presign API. (Fix simple typo in session command help #832)

- Avoid setting server encryption headers with x-amz-meta (minio#859) - Remove redirectHeaders method (minio#857) - Honor file pointer set in PutObject method (minio#856) - Fix docs and add a validator tool for our api docs (minio#850) - requestMetadata should take the md5 and sha256 with expected encodings. (minio#852) - Added a new method SetUserMetadata to the PostPolicy (minio#839) - Do not need to strictly validate request params in Presign API. (minio#832)

codecov-io · 2017-11-02T23:23:37Z

Codecov Report

Merging #2299 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff           @@
##           master   #2299   +/-   ##
======================================
  Coverage    10.7%   10.7%           
======================================
  Files         102     102           
  Lines        9916    9916           
======================================
  Hits         1062    1062           
  Misses       8716    8716           
  Partials      138     138

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab6b2a8...d851adf. Read the comment docs.

kannappanr

Verified basic commands manually. LGTM

ebozduman · 2017-11-06T22:48:35Z

vendor/github.com/minio/minio-go/api-put-object-common.go

@@ -40,6 +34,26 @@ func isObject(reader io.Reader) (ok bool) {
 // Verify if reader is a generic ReaderAt
 func isReadAt(reader io.Reader) (ok bool) {
 	_, ok = reader.(io.ReaderAt)
+	if ok {


if ok { => if _, ok = reader.(io.ReaderAt); ok {

ebozduman · 2017-11-06T22:49:51Z

vendor/github.com/minio/minio-go/api-put-object-common.go

+	if ok {
+		var v *os.File
+		v, ok = reader.(*os.File)
+		if ok {


if ok { => if v, ok = reader.(*os.File); ok {

ebozduman · 2017-11-06T23:03:29Z

vendor/github.com/minio/minio-go/api-put-object-streaming.go

-	// and such a functionality is used in the subsequent code path.
-	if isFile(reader) || !isObject(reader) && isReadAt(reader) {
+	if !isObject(reader) && isReadAt(reader) {
+		// Verify if the reader implements ReadAt and it is not a *minio.Object then we will use parallel uploader.


Don't we need to have the comment come before the command, like:

// Verify if the reader implements ReadAt and it is not a *minio.Object then we will use parallel uploader. if !isObject(reader) && isReadAt(reader) {

ebozduman · 2017-11-06T23:24:12Z

vendor/github.com/minio/minio-go/api-put-object-common.go

+				"/dev/stdin",
+				"/dev/stdout",
+				"/dev/stderr",
+			} {


IMHO, this looks much better, and more readable:

for _, f := range []string{"/dev/stdin", "/dev/stdout", "/dev/stderr"} {

This formatting is automatic by gofmt and this PR has nothing to do with these chnages, so won't be making these changes.

ebozduman · 2017-11-06T23:53:07Z

vendor/github.com/minio/minio-go/api.go

@@ -237,8 +223,7 @@ func privateNew(endpoint string, creds *credentials.Credentials, secure bool, re

 	// Instantiate http client and bucket location cache.
 	clnt.httpClient = &http.Client{
-		Transport:     defaultMinioTransport,
-		CheckRedirect: redirectHeaders,
+		Transport: defaultMinioTransport,


We can combine all in one line:

clnt.httpClient = &http.Client{Transport: defaultMinioTransport}

ebozduman · 2017-11-07T00:03:05Z

vendor/github.com/minio/minio-go/pkg/credentials/iam_aws.go

@@ -74,8 +62,7 @@ func NewIAM(endpoint string) *Credentials {
 	}
 	p := &IAM{
 		Client: &http.Client{
-			Transport:     http.DefaultTransport,
-			CheckRedirect: redirectHeaders,
+			Transport: http.DefaultTransport,


Combine them all in 2 lines:

p := &IAM { Client: &http.Client{Transport: http.DefaultTransport}, endpoint: endpoint }

ebozduman · 2017-11-07T00:10:27Z

vendor/github.com/minio/minio-go/pkg/s3signer/request-signature-v2.go

@@ -253,17 +251,27 @@ func writeCanonicalizedHeaders(buf *bytes.Buffer, req http.Request) {
 	}
 }

-// The following list is already sorted and should always be, otherwise we could
-// have signature-related issues
+// AWS S3 Signature V2 calculation rule is give here:


typo in comment:
// AWS S3 Signature V2 calculation rule is give here:
=???=> given

ebozduman · 2017-11-07T00:49:55Z

vendor/github.com/minio/minio-go/post-policy.go

+// SetUserMetadata - Set user metadata as a key/value couple.
+// Can be retrieved through a HEAD request or an event.
+func (p *PostPolicy) SetUserMetadata(key string, value string) error {
+	if strings.TrimSpace(key) == "" || key == "" {


strings.TrimSpace(key) for key="" returns "".
So, the 2nd condition is covered in the 1st part.
Then we can simplify the if condition as:

if strings.TrimSpace(key) == "" {

ebozduman · 2017-11-07T00:57:25Z

vendor/github.com/minio/minio-go/utils.go

@@ -38,18 +40,18 @@ func xmlDecoder(body io.Reader, v interface{}) error {
 	return d.Decode(v)
 }

-// sum256 calculate sha256 sum for an input byte array.
-func sum256(data []byte) []byte {
+// sum256 calculate sha256sum for an input byte array, returns hex encoded.


// sum256 calculate sha256sum ... => // sum256Hex calculates sha256sum ...`

harshavardhana · 2017-11-07T05:18:30Z

I cannot make your changes in this PR @ebozduman you should open them separately in minio-go this is Just a vendorized changes from minio-go master.

deekoder requested review from ebozduman and kannappanr November 3, 2017 02:33

kannappanr approved these changes Nov 3, 2017

View reviewed changes

ebozduman requested changes Nov 7, 2017

View reviewed changes

ebozduman approved these changes Nov 7, 2017

View reviewed changes

deekoder merged commit 014d094 into minio:master Nov 8, 2017

harshavardhana deleted the fix-minio-go branch November 8, 2017 04:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update minio-go #2299

Update minio-go #2299

harshavardhana commented Nov 2, 2017

codecov-io commented Nov 2, 2017

kannappanr left a comment

ebozduman Nov 6, 2017

ebozduman Nov 6, 2017

ebozduman Nov 6, 2017

ebozduman Nov 6, 2017

harshavardhana Nov 7, 2017

ebozduman Nov 6, 2017

ebozduman Nov 7, 2017

ebozduman Nov 7, 2017

ebozduman Nov 7, 2017

ebozduman Nov 7, 2017

harshavardhana commented Nov 7, 2017 •

edited

Loading

Update minio-go #2299

Update minio-go #2299

Conversation

harshavardhana commented Nov 2, 2017

codecov-io commented Nov 2, 2017

Codecov Report

kannappanr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshavardhana commented Nov 7, 2017 • edited Loading

harshavardhana commented Nov 7, 2017 •

edited

Loading