New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cat: Ignore Stat() error and pass to Get() #2166
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -497,40 +497,27 @@ func (c *s3Client) Watch(params watchParams) (*watchObject, *probe.Error) { | |
} | ||
|
||
// Get - get object with metadata. | ||
func (c *s3Client) Get() (io.Reader, map[string][]string, *probe.Error) { | ||
func (c *s3Client) Get() (io.Reader, *probe.Error) { | ||
bucket, object := c.url2BucketAndObject() | ||
reader, e := c.api.GetObject(bucket, object) | ||
if e != nil { | ||
errResponse := minio.ToErrorResponse(e) | ||
if errResponse.Code == "NoSuchBucket" { | ||
return nil, nil, probe.NewError(BucketDoesNotExist{ | ||
return nil, probe.NewError(BucketDoesNotExist{ | ||
Bucket: bucket, | ||
}) | ||
} | ||
if errResponse.Code == "InvalidBucketName" { | ||
return nil, nil, probe.NewError(BucketInvalid{ | ||
return nil, probe.NewError(BucketInvalid{ | ||
Bucket: bucket, | ||
}) | ||
} | ||
if errResponse.Code == "NoSuchKey" || errResponse.Code == "InvalidArgument" { | ||
return nil, nil, probe.NewError(ObjectMissing{}) | ||
} | ||
return nil, nil, probe.NewError(e) | ||
} | ||
objInfo, e := reader.Stat() | ||
if e != nil { | ||
errResponse := minio.ToErrorResponse(e) | ||
if errResponse.Code == "AccessDenied" { | ||
return nil, nil, probe.NewError(PathInsufficientPermission{Path: c.targetURL.String()}) | ||
} | ||
if errResponse.Code == "NoSuchKey" || errResponse.Code == "InvalidArgument" { | ||
return nil, nil, probe.NewError(ObjectMissing{}) | ||
return nil, probe.NewError(ObjectMissing{}) | ||
} | ||
return nil, nil, probe.NewError(e) | ||
return nil, probe.NewError(e) | ||
} | ||
metadata := objInfo.Metadata | ||
metadata.Set("Content-Type", objInfo.ContentType) | ||
return reader, metadata, nil | ||
return reader, nil | ||
} | ||
|
||
// Copy - copy object | ||
|
@@ -849,6 +836,7 @@ func (c *s3Client) Stat(isIncomplete bool) (*clientContent, *probe.Error) { | |
bucketMetadata := &clientContent{} | ||
bucketMetadata.URL = *c.targetURL | ||
bucketMetadata.Type = os.ModeDir | ||
bucketMetadata.Metadata = map[string][]string{} | ||
|
||
return bucketMetadata, nil | ||
} | ||
|
@@ -872,12 +860,14 @@ func (c *s3Client) Stat(isIncomplete bool) (*clientContent, *probe.Error) { | |
objectMetadata.Time = objectMultipartInfo.Initiated | ||
objectMetadata.Size = objectMultipartInfo.Size | ||
objectMetadata.Type = os.FileMode(0664) | ||
objectMetadata.Metadata = map[string][]string{} | ||
return objectMetadata, nil | ||
} | ||
|
||
if strings.HasSuffix(objectMultipartInfo.Key, string(c.targetURL.Separator)) { | ||
objectMetadata.URL = *c.targetURL | ||
objectMetadata.Type = os.ModeDir | ||
objectMetadata.Metadata = map[string][]string{} | ||
return objectMetadata, nil | ||
} | ||
} | ||
|
@@ -894,12 +884,14 @@ func (c *s3Client) Stat(isIncomplete bool) (*clientContent, *probe.Error) { | |
objectMetadata.Time = objectStat.LastModified | ||
objectMetadata.Size = objectStat.Size | ||
objectMetadata.Type = os.FileMode(0664) | ||
objectMetadata.Metadata = map[string][]string{} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why should we try to find the given object by listing the objects in the bucket for fetching stat information? We can directly call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @krisis, Stat() in client-s3 is supposed to be able to Stat() a directory but StatObject() of minio-go cannot. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @vadmeste Ah, didn't see that. Thanks for the explanation. |
||
return objectMetadata, nil | ||
} | ||
|
||
if strings.HasSuffix(objectStat.Key, string(c.targetURL.Separator)) { | ||
objectMetadata.URL = *c.targetURL | ||
objectMetadata.Type = os.ModeDir | ||
objectMetadata.Metadata = map[string][]string{} | ||
return objectMetadata, nil | ||
} | ||
} | ||
|
@@ -928,6 +920,11 @@ func (c *s3Client) Stat(isIncomplete bool) (*clientContent, *probe.Error) { | |
objectMetadata.Time = objectStat.LastModified | ||
objectMetadata.Size = objectStat.Size | ||
objectMetadata.Type = os.FileMode(0664) | ||
|
||
metadata := objectStat.Metadata | ||
metadata.Set("Content-Type", objectStat.ContentType) | ||
objectMetadata.Metadata = metadata | ||
|
||
return objectMetadata, nil | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, we return immediately if
err != nil
fromurl2Stat
. Why are we ignoring the error in this change?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the first place, url2Stat() is only needed to fetch the size of the remote object so we can check that we downloaded all the object data from the server. So actually, testing on url2Stat() is not crucial since we call getSourceStreamFromURL() later.
But why we ignore that error now? it is because we need to enhance support of mc with the new minio cache gateway. In some cases, with the cache gateway, a HEAD object returns 404 but GET can return 200 with object data. Refer to #2156 or ping me online.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vadmeste, OK. I am withholding approval of PR pending test. Otherwise, the change looks good to me.