Repository: peak/s5cmd
Pull request #115 (2020-03-24 07:41:37)

storage: Remove maxKeys parameter from List
We are using a full scan mode on storage.List() operations and do not need maxKeys param for this release. It was used by s3 predictor. Once we decided to reimplement it, we can add this parameter again. 


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 76d3369..9ab7e29 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,7 +4,7 @@
 
 This is a major release with many breaking changes.
 
-#### Backwards incompatible changes
+#### Breaking changes
 
 - Dropped `get` command. Users could get the same effect with `s5cmd cp <src> .`.
 - Dropped `nested command` support.
@@ -24,24 +24,24 @@ This is a major release with many breaking changes.
   resolving](https://aws.amazon.com/blogs/aws/amazon-s3-path-deprecation-plan-the-rest-of-the-story/)
   is enabled by default for S3 and GCS. If you provide a custom endpoint via
   `--endpoint-url` flag (other than GCS and S3 transfer acceleration), `s5cmd`
-  will fall back to the `path-style`. See [#92](https://github.com/peak/s5cmd/pull/92).
-- Listing a non-existent object will return exit code `1`, instead of `0`. See [#23](https://github.com/peak/s5cmd/issues/23).
+  will fall back to the `path-style`. ([#92](https://github.com/peak/s5cmd/pull/92))
+- Listing a non-existent object will return exit code `1`, instead of `0`. ([#23](https://github.com/peak/s5cmd/issues/23))
 
 #### Features
 
-- Added `mb` command to make buckets. See [#25](https://github.com/peak/s5cmd/issues/25).
-- Added `--json` flag for JSON logging. See [#22](https://github.com/peak/s5cmd/issues/22).
-- Added [S3 transfer acceleration](https://docs.aws.amazon.com/AmazonS3/latest/dev/transfer-acceleration.html) support. See [#40](https://github.com/peak/s5cmd/issues/40).
+- Added `mb` command to make buckets. ([#25](https://github.com/peak/s5cmd/issues/25))
+- Added `--json` flag for JSON logging. ([#22](https://github.com/peak/s5cmd/issues/22))
+- Added [S3 transfer acceleration](https://docs.aws.amazon.com/AmazonS3/latest/dev/transfer-acceleration.html) support. ([#40](https://github.com/peak/s5cmd/issues/40))
 
 #### Bugfixes
 
-- Correctly set `Content-Type` of a file on upload operations. See [#33](https://github.com/peak/s5cmd/issues/33).
+- Correctly set `Content-Type` of a file on upload operations. ([#33](https://github.com/peak/s5cmd/issues/33))
 
 #### Improvements
 
-- Pre-compiled binaries are provided on [releases page](https://github.com/peak/s5cmd/releases). See [#21](https://github.com/peak/s5cmd/issues/21).
-- AWS Go SDK is updated to support IAM role for service accounts. See [#32](https://github.com/peak/s5cmd/issues/32).
-- `s5cmd` now creates destination directory if missing.
+- Pre-compiled binaries are provided on [releases page](https://github.com/peak/s5cmd/releases). ([#21](https://github.com/peak/s5cmd/issues/21))
+- AWS Go SDK is updated to support IAM role for service accounts. ([#32](https://github.com/peak/s5cmd/issues/32))
+- For copy/move operations, `s5cmd` now creates destination directory if missing.
 
 ## v0.7.0 - 27 Jan 2020
 
diff --git a/command/cmd_copy.go b/command/cmd_copy.go
index b23b486..eaf5ee7 100644
--- a/command/cmd_copy.go
+++ b/command/cmd_copy.go
@@ -625,7 +625,7 @@ func expandSource(
 
 	// call storage.List for only walking operations.
 	if srcurl.HasGlob() || isDir {
-		return client.List(ctx, srcurl, isRecursive, storage.ListAllItems), nil
+		return client.List(ctx, srcurl, isRecursive), nil
 	}
 
 	ch := make(chan *storage.Object, 1)
diff --git a/command/cmd_list.go b/command/cmd_list.go
index 9d02fa6..97ad023 100644
--- a/command/cmd_list.go
+++ b/command/cmd_list.go
@@ -92,7 +92,7 @@ func List(
 
 	var merror error
 
-	for object := range client.List(ctx, srcurl, true, storage.ListAllItems) {
+	for object := range client.List(ctx, srcurl, true) {
 		if errorpkg.IsCancelation(object.Err) {
 			continue
 		}
diff --git a/command/cmd_size.go b/command/cmd_size.go
index 54eb7d0..6b4ce31 100644
--- a/command/cmd_size.go
+++ b/command/cmd_size.go
@@ -72,7 +72,7 @@ func Size(
 
 	var merror error
 
-	for object := range client.List(ctx, srcurl, true, storage.ListAllItems) {
+	for object := range client.List(ctx, srcurl, true) {
 		if object.Type.IsDir() || errorpkg.IsCancelation(object.Err) {
 			continue
 		}
diff --git a/storage/fs.go b/storage/fs.go
index 685ac5a..57206b3 100644
--- a/storage/fs.go
+++ b/storage/fs.go
@@ -39,7 +39,7 @@ func (f *Filesystem) Stat(ctx context.Context, url *objurl.ObjectURL) (*Object,
 	}, nil
 }
 
-func (f *Filesystem) List(ctx context.Context, url *objurl.ObjectURL, isRecursive bool, _ int64) <-chan *Object {
+func (f *Filesystem) List(ctx context.Context, url *objurl.ObjectURL, isRecursive bool) <-chan *Object {
 	obj, err := f.Stat(ctx, url)
 	isDir := err == nil && obj.Type.IsDir()
 
diff --git a/storage/s3.go b/storage/s3.go
index 700e25b..d4bd564 100644
--- a/storage/s3.go
+++ b/storage/s3.go
@@ -29,9 +29,6 @@ import (
 var _ Storage = (*S3)(nil)
 
 const (
-	// ListAllItems is a type to paginate all S3 keys.
-	ListAllItems = -1
-
 	// deleteObjectsMax is the max allowed objects to be deleted on single HTTP
 	// request.
 	deleteObjectsMax = 1000
@@ -143,7 +140,7 @@ func (s *S3) Stat(ctx context.Context, url *objurl.ObjectURL) (*Object, error) {
 // List is a non-blocking S3 list operation which paginates and filters S3
 // keys. If no object found or an error is encountered during this period,
 // it sends these errors to object channel.
-func (s *S3) List(ctx context.Context, url *objurl.ObjectURL, _ bool, maxKeys int64) <-chan *Object {
+func (s *S3) List(ctx context.Context, url *objurl.ObjectURL, _ bool) <-chan *Object {
 	listInput := s3.ListObjectsV2Input{
 		Bucket: aws.String(url.Bucket),
 		Prefix: aws.String(url.Prefix),
@@ -153,11 +150,6 @@ func (s *S3) List(ctx context.Context, url *objurl.ObjectURL, _ bool, maxKeys in
 		listInput.SetDelimiter(url.Delimiter)
 	}
 
-	shouldPaginate := maxKeys < 0
-	if !shouldPaginate {
-		listInput.SetMaxKeys(maxKeys)
-	}
-
 	objCh := make(chan *Object)
 
 	go func() {
@@ -208,7 +200,7 @@ func (s *S3) List(ctx context.Context, url *objurl.ObjectURL, _ bool, maxKeys in
 				objectFound = true
 			}
 
-			return shouldPaginate && !lastPage
+			return !lastPage
 		})
 
 		if err != nil {
diff --git a/storage/storage.go b/storage/storage.go
index 92fb57d..9de9837 100644
--- a/storage/storage.go
+++ b/storage/storage.go
@@ -30,7 +30,7 @@ type Storage interface {
 	// List the objects and directories/prefixes in the src. If recursive
 	// argument is given, given src will be walked if src is a walkable URL,
 	// such as directory, prefix or a wildcard.
-	List(ctx context.Context, src *objurl.ObjectURL, recursive bool, maxitems int64) <-chan *Object
+	List(ctx context.Context, src *objurl.ObjectURL, recursive bool) <-chan *Object
 
 	// Copy src to dst, optionally setting the given metadata. Src and dst
 	// arguments are of the same type. If src is a remote type, server side


Test patch:
diff --git a/storage/s3_test.go b/storage/s3_test.go
index 51d9afe..edb51f2 100644
--- a/storage/s3_test.go
+++ b/storage/s3_test.go
@@ -145,7 +145,7 @@ func TestS3_List_success(t *testing.T) {
 	}
 
 	index := 0
-	for got := range mockS3.List(context.Background(), url, true, ListAllItems) {
+	for got := range mockS3.List(context.Background(), url, true) {
 		if got.Err != nil {
 			t.Errorf("unexpected error: %v", got.Err)
 		}
@@ -184,7 +184,7 @@ func TestS3_List_error(t *testing.T) {
 		r.Error = mockErr
 	})
 
-	for got := range mockS3.List(context.Background(), url, true, ListAllItems) {
+	for got := range mockS3.List(context.Background(), url, true) {
 		if got.Err != mockErr {
 			t.Errorf("error got = %v, want %v", got.Err, mockErr)
 		}
@@ -221,7 +221,7 @@ func TestS3_List_no_item_found(t *testing.T) {
 		}
 	})
 
-	for got := range mockS3.List(context.Background(), url, true, ListAllItems) {
+	for got := range mockS3.List(context.Background(), url, true) {
 		if got.Err != ErrNoObjectFound {
 			t.Errorf("error got = %v, want %v", got.Err, ErrNoObjectFound)
 		}
@@ -255,7 +255,7 @@ func TestS3_List_context_cancelled(t *testing.T) {
 		}
 	})
 
-	for got := range mockS3.List(ctx, url, true, ListAllItems) {
+	for got := range mockS3.List(ctx, url, true) {
 		reqErr, ok := got.Err.(awserr.Error)
 		if !ok {
 			t.Errorf("could not convert error")