Add central log package (peak#80)

Tarifut · Mar 2, 2020 · 83edca8 · 83edca8
1 parent 9224689
commit 83edca8
Show file tree

Hide file tree

Showing 12 changed files with 262 additions and 158 deletions.
diff --git a/Makefile b/Makefile
@@ -3,8 +3,7 @@ SRCDIR ?= .
 default: all
 
 .PHONY: all
-all: clean build test vet staticcheck check-fmt
-
+all: clean build test check
 .PHONY: dist
 dist: generate all
 
@@ -24,6 +23,9 @@ build:
 test:
 	@go test -mod=vendor ./...
 
+.PHONY: check
+check: vet staticcheck check-fmt
+
 .PHONY: staticcheck
 staticcheck:
 	@staticcheck -checks 'inherit,-SA4009,-U1000' ./...

diff --git a/core/error.go b/core/error.go
diff --git a/core/job.go b/core/job.go
@@ -3,11 +3,11 @@ package core
 import (
 	"context"
 	"errors"
-	"fmt"
-	"log"
+	stdlog "log"
 
 	"github.com/hashicorp/go-multierror"
-	"github.com/peak/s5cmd/flags"
+
+	"github.com/peak/s5cmd/log"
 	"github.com/peak/s5cmd/objurl"
 	"github.com/peak/s5cmd/op"
 	"github.com/peak/s5cmd/opt"
@@ -28,23 +28,21 @@ type Job struct {
 	args         []*objurl.ObjectURL
 	storageClass storage.StorageClass
 	command      string
-	response     *JobResponse
 	statType     stats.StatType
 }
 
 // JobResponse is the response type.
 type JobResponse struct {
-	status  JobStatus
-	message []string
-	err     error
+	status JobStatus
+	err    error
 }
 
 // jobResponse creates a new JobResponse by setting job status, message and error.
-func jobResponse(err error, msg ...string) *JobResponse {
+func jobResponse(err error) *JobResponse {
 	if err == nil {
-		return &JobResponse{status: statusSuccess, message: msg}
+		return &JobResponse{status: statusSuccess}
 	}
-	return &JobResponse{status: statusErr, err: err, message: msg}
+	return &JobResponse{status: statusErr, err: err}
 }
 
 // String formats the job using its command and arguments.
@@ -58,53 +56,29 @@ func (j Job) String() string {
 	return s
 }
 
-// Log prints the results of jobs.
-func (j *Job) Log() {
-	status := j.response.status
-	err := j.response.err
-
-	for _, m := range j.response.message {
-		fmt.Println("                   ", status, m)
-	}
-
-	errStr := ""
-	if err != nil {
-		if !*flags.Verbose && isCancelationError(err) {
-			return
-		}
-
-		errStr = CleanupError(err)
-		errStr = fmt.Sprintf(" (%s)", errStr)
-	}
-
-	if status == statusErr {
-		log.Printf(`-ERR "%s": %s`, j, errStr)
-		return
-	}
-
-	m := fmt.Sprintf(`"%s"%s`, j, errStr)
-	if status != statusSuccess {
-		fmt.Println(status, m)
-	}
-}
-
 // Run runs the Job, gets job response and logs the job status.
 func (j *Job) Run(ctx context.Context) {
 	cmdFunc, ok := globalCmdRegistry[j.operation]
 	if !ok {
-		log.Fatalf("unhandled operation %v", j.operation)
+		// TODO(ig): log and continue
+		stdlog.Fatalf("unhandled operation %v", j.operation)
 		return
 	}
 
 	response := cmdFunc(ctx, j)
-	if response != nil {
-		if response.status == statusErr {
-			stats.Increment(stats.Fail)
-		} else {
-			stats.Increment(j.statType)
-		}
-		j.response = response
-		j.Log()
+	if response == nil {
+		return
+	}
+
+	switch response.status {
+	case statusErr:
+		stats.Increment(stats.Fail)
+		log.Logger.Error("%q: %v", j, response.err)
+	case statusWarning:
+		log.Logger.Warning("%q (%v)", j, response.err)
+		fallthrough
+	default:
+		stats.Increment(j.statType)
 	}
 }
 

diff --git a/core/job_exec.go b/core/job_exec.go
@@ -7,6 +7,7 @@ import (
 	"strings"
 
 	"github.com/hashicorp/go-multierror"
+	"github.com/peak/s5cmd/log"
 	"github.com/peak/s5cmd/objurl"
 	"github.com/peak/s5cmd/opt"
 	"github.com/peak/s5cmd/storage"
@@ -25,7 +26,7 @@ func Copy(ctx context.Context, job *Job) *JobResponse {
 		return jobResponse(err)
 	}
 
-	infoLog("Copying %v...", src.Base())
+	log.Logger.Info("Copying %v...", src.Base())
 
 	metadata := map[string]string{
 		"StorageClass": string(job.storageClass),
@@ -53,7 +54,7 @@ func Delete(ctx context.Context, job *Job) *JobResponse {
 		return jobResponse(err)
 	}
 
-	infoLog("Deleting %v...", src.Base())
+	log.Logger.Info("Deleting %v...", src.Base())
 
 	err = client.Delete(ctx, src)
 	return jobResponse(err)
@@ -77,7 +78,6 @@ func Download(ctx context.Context, job *Job) *JobResponse {
 		return jobResponse(err)
 	}
 
-	srcFilename := src.Base()
 	destFilename := dst.Absolute()
 
 	// TODO(ig): use storage abstraction
@@ -87,7 +87,7 @@ func Download(ctx context.Context, job *Job) *JobResponse {
 	}
 	defer f.Close()
 
-	infoLog("Downloading %s...", srcFilename)
+	log.Logger.Info("Downloading %v...", src.Base())
 
 	err = srcClient.Get(ctx, src, f)
 	if err != nil {
@@ -124,8 +124,7 @@ func Upload(ctx context.Context, job *Job) *JobResponse {
 		return jobResponse(err)
 	}
 
-	srcFilename := src.Base()
-	infoLog("Uploading %s...", srcFilename)
+	log.Logger.Info("Uploading %v...", src.Base())
 
 	metadata := map[string]string{
 		"StorageClass": string(job.storageClass),
@@ -187,20 +186,21 @@ func BatchDelete(ctx context.Context, job *Job) *JobResponse {
 
 	// closed errch indicates that MultiDelete operation is finished.
 	var merror error
-	var msg []string
 	for obj := range resultch {
 		if obj.Err != nil {
 			merror = multierror.Append(merror, err)
-			msg = append(msg, fmt.Sprintf(`Batch-delete %v: %v`, obj.URL, err))
-		} else {
-			msg = append(msg, fmt.Sprintf("Batch-delete %v", obj.URL))
+
+			log.Logger.Error("%q: %v", job, err)
+			continue
 		}
+
+		log.Logger.Success("Batch-delete %v", obj.URL)
 	}
 
-	return jobResponse(merror, msg...)
+	return jobResponse(merror)
 }
 
-func ListBuckets(ctx context.Context, _ *Job) *JobResponse {
+func ListBuckets(ctx context.Context, job *Job) *JobResponse {
 	// set as remote storage
 	url := &objurl.ObjectURL{Type: 0}
 	client, err := storage.NewClient(url)
@@ -213,12 +213,11 @@ func ListBuckets(ctx context.Context, _ *Job) *JobResponse {
 		return jobResponse(err)
 	}
 
-	var msg []string
 	for _, b := range buckets {
-		msg = append(msg, b.String())
+		log.Logger.Success(b.String())
 	}
 
-	return jobResponse(err, msg...)
+	return jobResponse(err)
 }
 
 func List(ctx context.Context, job *Job) *JobResponse {
@@ -232,15 +231,22 @@ func List(ctx context.Context, job *Job) *JobResponse {
 		return jobResponse(err)
 	}
 
-	var msg []string
 	for object := range client.List(ctx, src, true, storage.ListAllItems) {
 		if object.Err != nil {
 			// TODO(ig): expose or log the error
 			continue
 		}
 
 		if object.Mode.IsDir() {
-			msg = append(msg, fmt.Sprintf("%19s %1s %-38s  %12s  %s", "", "", "", "DIR", object.URL.Relative()))
+			s := fmt.Sprintf(
+				"%19s %1s %-38s  %12s  %s",
+				"",
+				"",
+				"",
+				"DIR",
+				object.URL.Relative(),
+			)
+			log.Logger.Success(s)
 			continue
 		}
 
@@ -255,19 +261,18 @@ func List(ctx context.Context, job *Job) *JobResponse {
 			size = fmt.Sprintf("%d", object.Size)
 		}
 
-		msg = append(
-			msg,
-			fmt.Sprintf("%s %1s %-38s %12s  %s",
-				object.ModTime.Format(dateFormat),
-				object.StorageClass.ShortCode(),
-				etag,
-				size,
-				object.URL.Relative(),
-			),
+		s := fmt.Sprintf(
+			"%19s %1s %-38s  %12s  %s",
+			object.ModTime.Format(dateFormat),
+			object.StorageClass.ShortCode(),
+			etag,
+			size,
+			object.URL.Relative(),
 		)
+		log.Logger.Success(s)
 	}
 
-	return jobResponse(nil, msg...)
+	return jobResponse(nil)
 }
 
 func Size(ctx context.Context, job *Job) *JobResponse {
@@ -306,14 +311,16 @@ func Size(ctx context.Context, job *Job) *JobResponse {
 		totals["Total"] = sz
 	}
 
-	var msg []string
 	for k, v := range totals {
+		var msg string
 		if job.opts.Has(opt.HumanReadable) {
-			msg = append(msg, fmt.Sprintf("%s bytes in %d objects: %s [%s]", HumanizeBytes(v.size), v.count, src, k))
+			msg = fmt.Sprintf("%s bytes in %d objects: %s [%s]", HumanizeBytes(v.size), v.count, src, k)
 		} else {
-			msg = append(msg, fmt.Sprintf("%d bytes in %d objects: %s [%s]", v.size, v.count, src, k))
+			msg = fmt.Sprintf("%d bytes in %d objects: %s [%s]", v.size, v.count, src, k)
 		}
+
+		log.Logger.Success(msg)
 	}
 
-	return jobResponse(err, msg...)
+	return jobResponse(err)
 }
diff --git a/core/job_status.go b/core/job_status.go
@@ -13,20 +13,6 @@ const (
 	statusWarning
 )
 
-// String returns the string representation of JobStatus.
-func (s JobStatus) String() string {
-	switch s {
-	case statusSuccess:
-		return "+"
-	case statusErr:
-		return "ERROR"
-	case statusWarning:
-		return "WARNING"
-	default:
-		return "UNKNOWN"
-	}
-}
-
 //  OK-to-have error types (warnings) that is used when the job status is warning.
 var (
 	ErrObjectExists     = fmt.Errorf("object already exists")

diff --git a/core/log.go b/core/log.go