Skip to content

Conversation

@dixia
Copy link

@dixia dixia commented Apr 21, 2017

Support larger list of version and AWS API v4

Support larger list of version and AWS API v4
@abhayachauhan abhayachauhan self-requested a review April 24, 2017 00:45
@abhayachauhan abhayachauhan self-assigned this Apr 24, 2017
@abhayachauhan abhayachauhan removed their request for review April 24, 2017 00:45
@dixia
Copy link
Author

dixia commented Apr 26, 2017

This is a small problem with this PR. I will make a better version later.

s3DataContents = s3DataContents.concat(contents.DeleteMarkers);
if (data.IsTruncated) {
params.VersionIdMarker = contents.NextVersionIdMarker;
params.KeyMarker = contents.NextVersionIdMarker;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this line not be
params.KeyMarker = contents.NextKeyMarker;


s3 = new AWS.S3();
s3 = new AWS.S3(
{signatureVersion: 'v4'}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under the impression AWS SDK handles this automatically, although I may have missed something - can you explain why we need to specify the signatureVersion to v4?

params.KeyMarker = contents.NextVersionIdMarker;
recursiveCall(params);
} else {
s3DataContents = s3DataContents.filter((elm) => { return elm.Size > 0 })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We cannot do this - this will remove items which need to be deleted.

@abhayachauhan
Copy link
Contributor

Sorry about the delay in reviewing. I've made some comments, and also fixed up tests to run on travis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants