command/, storage/: updates based on pr review

peak · igungor · Oct 22, 2020 · Jul 16, 2020 · Jul 17, 2020 · Jul 17, 2020
commit b50bed95516ba5faf3bb16076e05ae86a1f0892d
@@ -9,7 +9,6 @@ import (
 
 	"github.com/peak/s5cmd/log"
 	"github.com/peak/s5cmd/parallel"
-	"github.com/peak/s5cmd/storage"
 )
 
 const (
@@ -19,10 +18,6 @@ const (
 	appName = "s5cmd"
 )
 
-// AppStorageOptions will be overridden by inner command flags
-// such as if provided `cp -region` will override s5cmd -region flag value.
-var AppStorageOptions storage.StorageOptions
-
 var app = &cli.App{
 	Name:  appName,
 	Usage: "Blazing fast S3 and local filesystem execution tool",
@@ -59,19 +54,9 @@ var app = &cli.App{
 			Name:  "install-completion",
 			Usage: "install completion for your shell",
 		},
-		&cli.StringFlag{
-			Name:  "source-region",
-			Usage: "connect to a specific region of the remote object storage service",
-		},
-		&cli.StringFlag{
-			Name:  "region",
-			Usage: "(global) region of the destination bucket for cp/mv operations; default is source-region",
-		},
 	},
 	Before: func(c *cli.Context) error {
-		noVerifySSL := c.Bool("no-verify-ssl")
 		retryCount := c.Int("retry-count")
-		endpointURL := c.String("endpoint-url")
 		workerCount := c.Int("numworkers")
 		printJSON := c.Bool("json")
 		logLevel := c.String("log")
@@ -83,14 +68,6 @@ var app = &cli.App{
 			return fmt.Errorf("retry count cannot be a negative value")
 		}
 
-		AppStorageOptions = storage.StorageOptions{
-			MaxRetries:        retryCount,
-			Endpoint:          endpointURL,
-			NoVerifySSL:       noVerifySSL,
-			SourceRegion:      c.String("source-region"),
-			DestinationRegion: c.String("region"),
-		}
-
 		return nil
 	},
 	Action: func(c *cli.Context) error {

@@ -61,13 +61,13 @@ var catCommand = &cli.Command{
 			return err
 		}
 
-		return Cat(c.Context, src)
+		return Cat(c.Context, src, storage.NewS3Options(c, true))
 	},
 }
 
 // Cat prints content of given source to standard output.
-func Cat(ctx context.Context, src *url.URL) error {
-	client, err := storage.NewClient(src, AppStorageOptions)
+func Cat(ctx context.Context, src *url.URL, s3Opts storage.S3Options) error {
+	client, err := storage.NewClient(src, s3Opts)
 	if err != nil {
 		return err
 	}

@@ -148,20 +148,9 @@ var copyCommand = &cli.Command{
 	Flags:              copyCommandFlags,
 	CustomHelpTemplate: copyHelpTemplate,
 	Before: func(c *cli.Context) error {
-		return validate(c, AppStorageOptions)
+		return validate(c, storage.NewS3Options(c, true))
 	},
 	Action: func(c *cli.Context) error {
-
-		// get application level values for the flags if not provided
-		srcRegion := c.String("source-region")
-		if srcRegion == "" {
-			srcRegion = AppStorageOptions.SourceRegion
-		}
-		dstRegion := c.String("region")
-		if dstRegion == "" {
-			dstRegion = AppStorageOptions.DestinationRegion
-		}
-
 		return Copy{
 			src:          c.Args().Get(0),
 			dst:          c.Args().Get(1),
@@ -179,15 +168,8 @@ var copyCommand = &cli.Command{
 			encryptionKeyID:  c.String("sse-kms-key-id"),
 			acl:              c.String("acl"),
 
-			StorageOptions: storage.StorageOptions{
-				Concurrency:       c.Int("concurrency"),
-				PartSize:          c.Int64("part-size") * megabytes,
-				SourceRegion:      srcRegion,
-				DestinationRegion: dstRegion,
-				MaxRetries:        AppStorageOptions.MaxRetries,
-				NoVerifySSL:       AppStorageOptions.NoVerifySSL,
-				Endpoint:          AppStorageOptions.Endpoint,
-			},
+			srcS3opts: storage.NewS3Options(c, true),
+			dstS3opts: storage.NewS3Options(c, false),
 		}.Run(c.Context)
 	},
 }
@@ -213,7 +195,10 @@ type Copy struct {
 	acl              string
 
 	// s3 options
-	storage.StorageOptions
+	concurrency int
+	partSize    int64
+	srcS3opts   storage.S3Options
+	dstS3opts   storage.S3Options
 }
 
 const fdlimitWarning = `
@@ -234,7 +219,7 @@ func (c Copy) Run(ctx context.Context) error {
 		return err
 	}
 
-	client, err := storage.NewClient(srcurl, c.StorageOptions)
+	client, err := storage.NewClient(srcurl, c.srcS3opts)
 	if err != nil {
 		return err
 	}
@@ -336,7 +321,7 @@ func (c Copy) prepareDownloadTask(
 	isBatch bool,
 ) func() error {
 	return func() error {
-		dsturl, err := prepareLocalDestination(ctx, srcurl, dsturl, c.flatten, isBatch, c.StorageOptions)
+		dsturl, err := prepareLocalDestination(ctx, srcurl, dsturl, c.flatten, isBatch, c.dstS3opts)
 		if err != nil {
 			return err
 		}
@@ -377,11 +362,11 @@ func (c Copy) prepareUploadTask(
 
 // doDownload is used to fetch a remote object and save as a local object.
 func (c Copy) doDownload(ctx context.Context, srcurl *url.URL, dsturl *url.URL) error {
-	srcClient, err := storage.NewClient(srcurl, c.StorageOptions)
+	srcClient, err := storage.NewClient(srcurl, c.srcS3opts)
 	if err != nil {
 		return err
 	}
-	dstClient, err := storage.NewClient(dsturl, c.StorageOptions)
+	dstClient, err := storage.NewClient(dsturl, c.dstS3opts)
 	if err != nil {
 		return err
 	}
@@ -402,7 +387,7 @@ func (c Copy) doDownload(ctx context.Context, srcurl *url.URL, dsturl *url.URL)
 	}
 	defer f.Close()
 
-	size, err := srcClient.Get(ctx, srcurl, f, c.Concurrency, c.PartSize)
+	size, err := srcClient.Get(ctx, srcurl, f, c.concurrency, c.partSize)
 	if err != nil {
 		_ = dstClient.Delete(ctx, dsturl)
 		return err
@@ -442,7 +427,7 @@ func (c Copy) doUpload(ctx context.Context, srcurl *url.URL, dsturl *url.URL) er
 		return err
 	}
 
-	dstClient, err := storage.NewClient(dsturl, c.StorageOptions)
+	dstClient, err := storage.NewClient(dsturl, c.dstS3opts)
 	if err != nil {
 		return err
 	}
@@ -454,12 +439,12 @@ func (c Copy) doUpload(ctx context.Context, srcurl *url.URL, dsturl *url.URL) er
 		SetSSEKeyID(c.encryptionKeyID).
 		SetACL(c.acl)
 
-	err = dstClient.Put(ctx, f, dsturl, metadata, c.Concurrency, c.PartSize)
+	err = dstClient.Put(ctx, f, dsturl, metadata, c.concurrency, c.partSize)
 	if err != nil {
 		return err
 	}
 
-	srcClient, err := storage.NewClient(srcurl, c.StorageOptions)
+	srcClient, err := storage.NewClient(srcurl, c.srcS3opts)
 	if err != nil {
 		return err
 	}
@@ -490,7 +475,7 @@ func (c Copy) doUpload(ctx context.Context, srcurl *url.URL, dsturl *url.URL) er
 }
 
 func (c Copy) doCopy(ctx context.Context, srcurl *url.URL, dsturl *url.URL) error {
-	srcClient, err := storage.NewClient(srcurl, c.StorageOptions)
+	srcClient, err := storage.NewClient(srcurl, c.srcS3opts)
 	if err != nil {
 		return err
 	}
@@ -546,12 +531,12 @@ func (c Copy) shouldOverride(ctx context.Context, srcurl *url.URL, dsturl *url.U
 		return nil
 	}
 
-	srcObj, err := getObject(ctx, srcurl, c.StorageOptions)
+	srcObj, err := getObject(ctx, srcurl, c.srcS3opts)
 	if err != nil {
 		return err
 	}
 
-	dstObj, err := getObject(ctx, dsturl, c.StorageOptions)
+	dstObj, err := getObject(ctx, dsturl, c.dstS3opts)
 	if err != nil {
 		return err
 	}
@@ -614,7 +599,7 @@ func prepareLocalDestination(
 	dsturl *url.URL,
 	flatten bool,
 	isBatch bool,
-	storageOpts storage.StorageOptions,
+	s3opts storage.S3Options,
 ) (*url.URL, error) {
 	objname := srcurl.Base()
 	if isBatch && !flatten {
@@ -627,7 +612,7 @@ func prepareLocalDestination(
 		}
 	}
 
-	client, err := storage.NewClient(dsturl, storageOpts)
+	client, err := storage.NewClient(dsturl, s3opts)
 	if err != nil {
 		return nil, err
 	}
@@ -662,8 +647,8 @@ func prepareLocalDestination(
 
 // getObject checks if the object from given url exists. If no object is
 // found, error and returning object would be nil.
-func getObject(ctx context.Context, url *url.URL, storageOpts storage.StorageOptions) (*storage.Object, error) {
-	client, err := storage.NewClient(url, storageOpts)
+func getObject(ctx context.Context, url *url.URL, s3opts storage.S3Options) (*storage.Object, error) {
+	client, err := storage.NewClient(url, s3opts)
 	if err != nil {
 		return nil, err
 	}
@@ -676,7 +661,7 @@ func getObject(ctx context.Context, url *url.URL, storageOpts storage.StorageOpt
 	return obj, err
 }
 
-func validate(c *cli.Context, storageOpts storage.StorageOptions) error {
+func validate(c *cli.Context, srcS3opts storage.S3Options) error {
 	if c.Args().Len() != 2 {
 		return fmt.Errorf("expected source and destination arguments")
 	}
@@ -715,7 +700,7 @@ func validate(c *cli.Context, storageOpts storage.StorageOptions) error {
 	case srcurl.Type == dsturl.Type:
 		return validateCopy(srcurl, dsturl)
 	case dsturl.IsRemote():
-		return validateUpload(ctx, srcurl, dsturl, storageOpts)
+		return validateUpload(ctx, srcurl, dsturl, srcS3opts)
 	default:
 		return nil
 	}
@@ -730,8 +715,8 @@ func validateCopy(srcurl, dsturl *url.URL) error {
 	return fmt.Errorf("local->local copy operations are not permitted")
 }
 
-func validateUpload(ctx context.Context, srcurl, dsturl *url.URL, storageOpts storage.StorageOptions) error {
-	srcclient, err := storage.NewClient(srcurl, storageOpts)
+func validateUpload(ctx context.Context, srcurl, dsturl *url.URL, srcS3opts storage.S3Options) error {
+	srcclient, err := storage.NewClient(srcurl, srcS3opts)
 	if err != nil {
 		return err
 	}

@@ -63,6 +63,7 @@ var sizeCommand = &cli.Command{
 			c.Args().First(),
 			groupByClass,
 			humanize,
+			storage.NewS3Options(c, true),
 		)
 	},
 }
@@ -73,13 +74,14 @@ func Size(
 	src string,
 	groupByClass bool,
 	humanize bool,
+	s3opts storage.S3Options,
 ) error {
 	srcurl, err := url.New(src)
 	if err != nil {
 		return err
 	}
 
-	client, err := storage.NewClient(srcurl, AppStorageOptions)
+	client, err := storage.NewClient(srcurl, s3opts)
 	if err != nil {
 		return err
 	}

@@ -67,7 +67,7 @@ var listCommand = &cli.Command{
 	},
 	Action: func(c *cli.Context) error {
 		if !c.Args().Present() {
-			return ListBuckets(c.Context)
+			return ListBuckets(c.Context, storage.NewS3Options(c, true))
 		}
 
 		showEtag := c.Bool("etag")
@@ -80,15 +80,16 @@ var listCommand = &cli.Command{
 			showEtag,
 			humanize,
 			showStorageClass,
+			storage.NewS3Options(c, true),
 		)
 	},
 }
 
 // ListBuckets prints all buckets.
-func ListBuckets(ctx context.Context) error {
+func ListBuckets(ctx context.Context, s3opts storage.S3Options) error {
 	// set as remote storage
 	url := &url.URL{Type: 0}
-	client, err := storage.NewClient(url, AppStorageOptions)
+	client, err := storage.NewClient(url, s3opts)
 	if err != nil {
 		return err
 	}
@@ -113,13 +114,14 @@ func List(
 	showEtag bool,
 	humanize bool,
 	showStorageClass bool,
+	s3opts storage.S3Options,
 ) error {
 	srcurl, err := url.New(src)
 	if err != nil {
 		return err
 	}
 
-	client, err := storage.NewClient(srcurl, AppStorageOptions)
+	client, err := storage.NewClient(srcurl, s3opts)
 	if err != nil {
 		return err
 	}

@@ -51,6 +51,7 @@ var makeBucketCommand = &cli.Command{
 			c.Context,
 			c.Command.Name,
 			c.Args().First(),
+			storage.NewS3Options(c, true),
 		)
 	},
 }
@@ -60,13 +61,14 @@ func MakeBucket(
 	ctx context.Context,
 	op string,
 	src string,
+	s3opts storage.S3Options,
 ) error {
 	bucket, err := url.New(src)
 	if err != nil {
 		return err
 	}
 
-	client, err := storage.NewClient(bucket, AppStorageOptions)
+	client, err := storage.NewClient(bucket, s3opts)
 	if err != nil {
 		return err
 	}