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
Added ListObjectsV2 support to gateway #4547
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4547 +/- ##
==========================================
- Coverage 66.4% 66.22% -0.19%
==========================================
Files 179 179
Lines 25757 25903 +146
==========================================
+ Hits 17105 17155 +50
- Misses 7502 7596 +94
- Partials 1150 1152 +2
Continue to review full report at Codecov.
|
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.
Just remember to send another commit to use azureAnonRequest() once #4543 is merged.
MaxResults: uint(maxKeys), | ||
} | ||
|
||
q := azureListBlobsGetParameters(params) |
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.
We need to add unit tests for all this parsing logic.. and converting to S3 compatible format.
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.
Added tests for azureListBlobsGetParameters
. Will check if there are more such parsing methods, will add tests for those as well
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.
Added tests for anonErrToObjectErr
. Looks like there are no more parsing/response translation methods without unit tests in gateway-azure.go
cmd/gateway-azure.go
Outdated
@@ -269,6 +269,41 @@ func (a *azureObjects) ListObjects(bucket, prefix, marker, delimiter string, max | |||
return result, nil | |||
} | |||
|
|||
// ListObjectsV2 - lists all blobs in GCS bucket filtered by prefix |
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.
s/list all blobs in GCS bucket/list all blobs in Azure bucket/
cmd/gateway-handlers.go
Outdated
// marshalled into S3 compatible XML header. | ||
listObjectsV2Info, err := listObjectsV2(bucket, prefix, token, fetchOwner, delimiter, maxKeys) | ||
if err != nil { | ||
errorIf(err, "Unable to list objects.") |
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.
We could improve the error log by adding bucket
, prefix
, delimiter
and marker
. This will help debug when such an error does occur.
cmd/gateway-handlers.go
Outdated
} | ||
// Inititate a list objects operation based on the input params. | ||
// On success would return back ListObjectsInfo object to be | ||
// marshalled into S3 compatible XML header. |
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.
ListObjectsInfo is serialized as XML and is sent as part of the response body, not header.
Updated, @harshavardhana @krisis PTAL |
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.
LGTM. Tested locally and works as expected.
Description
Added ListObjectsV2 and ListObjectsV2 Anon support to Gateway S3 and Azure
Motivation and Context
How Has This Been Tested?
This adds anonymous list object V2 support to S3 gateway.
Methods ListObjectsV2and AnonListObjectsV2 are now added to GatewayLayer interface.
Partially fixes #4413. Will send another PR to
feature-gcs
branch.Types of changes
Checklist: