From 243343c0e9166ffdd644f6fb75ce12f02235806a Mon Sep 17 00:00:00 2001 From: Keepers <104464746+ryanfkeepers@users.noreply.github.com> Date: Fri, 17 Jun 2022 13:45:22 -0600 Subject: [PATCH] Replace fmt prints with the logger (#220) Now that we have the logger in place, we should use it in place of fmt.Print calls for development and debugging output. --- src/cli/backup/exchange.go | 26 ++++++++++++------- src/cli/repo/s3.go | 22 +++++++++++----- src/cli/restore/exchange.go | 25 +++++++++--------- src/cli/utils/utils.go | 14 ++++++++++ src/internal/connector/errors.go | 14 +++++----- src/internal/connector/errors_test.go | 5 ++-- src/internal/connector/graph_connector.go | 17 +++++++----- .../connector/graph_connector_test.go | 5 ++-- src/internal/operations/backup.go | 2 +- 9 files changed, 84 insertions(+), 46 deletions(-) diff --git a/src/cli/backup/exchange.go b/src/cli/backup/exchange.go index c2da42563..ceafe97fd 100644 --- a/src/cli/backup/exchange.go +++ b/src/cli/backup/exchange.go @@ -9,6 +9,7 @@ import ( "github.com/alcionai/corso/cli/config" "github.com/alcionai/corso/cli/utils" "github.com/alcionai/corso/pkg/credentials" + "github.com/alcionai/corso/pkg/logger" "github.com/alcionai/corso/pkg/repository" ) @@ -40,6 +41,12 @@ var exchangeCreateCmd = &cobra.Command{ // initializes a s3 repo. func createExchangeCmd(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + + if utils.HasNoFlagsAndShownHelp(cmd) { + return nil + } + s, cfgTenantID, err := config.MakeS3Config(true, nil) if err != nil { return err @@ -55,25 +62,24 @@ func createExchangeCmd(cmd *cobra.Command, args []string) error { a.TenantID = cfgTenantID } - fmt.Printf( - "Called - %s\n\t365TenantID:\t%s\n\t356Client:\t%s\n\tfound 356Secret:\t%v\n", - cmd.CommandPath(), - m365.TenantID, - m365.ClientID, - len(m365.ClientSecret) > 0) + logger.Ctx(ctx).Debugw( + "Called - "+cmd.CommandPath(), + "tenantID", m365.TenantID, + "clientID", m365.ClientID, + "hasClientSecret", len(m365.ClientSecret) > 0) - r, err := repository.Connect(cmd.Context(), a, s) + r, err := repository.Connect(ctx, a, s) if err != nil { return errors.Wrapf(err, "Failed to connect to the %s repository", s.Provider) } - defer utils.CloseRepo(cmd.Context(), r) + defer utils.CloseRepo(ctx, r) - bo, err := r.NewBackup(cmd.Context(), []string{user}) + bo, err := r.NewBackup(ctx, []string{user}) if err != nil { return errors.Wrap(err, "Failed to initialize Exchange backup") } - if _, err := bo.Run(cmd.Context()); err != nil { + if _, err := bo.Run(ctx); err != nil { return errors.Wrap(err, "Failed to run Exchange backup") } diff --git a/src/cli/repo/s3.go b/src/cli/repo/s3.go index 6ffbe833f..2fb45e9bf 100644 --- a/src/cli/repo/s3.go +++ b/src/cli/repo/s3.go @@ -52,7 +52,12 @@ var s3InitCmd = &cobra.Command{ // initializes a s3 repo. func initS3Cmd(cmd *cobra.Command, args []string) error { - log := logger.Ctx(cmd.Context()) + ctx := cmd.Context() + log := logger.Ctx(ctx) + + if utils.HasNoFlagsAndShownHelp(cmd) { + return nil + } overrides := map[string]string{ credentials.AWSAccessKeyID: accessKey, @@ -87,11 +92,11 @@ func initS3Cmd(cmd *cobra.Command, args []string) error { "accessKey", s3Cfg.AccessKey, "hasSecretKey", len(s3Cfg.SecretKey) > 0) - r, err := repository.Initialize(cmd.Context(), a, s) + r, err := repository.Initialize(ctx, a, s) if err != nil { return errors.Wrap(err, "Failed to initialize a new S3 repository") } - defer utils.CloseRepo(cmd.Context(), r) + defer utils.CloseRepo(ctx, r) fmt.Printf("Initialized a S3 repository within bucket %s.\n", s3Cfg.Bucket) @@ -112,7 +117,12 @@ var s3ConnectCmd = &cobra.Command{ // connects to an existing s3 repo. func connectS3Cmd(cmd *cobra.Command, args []string) error { - log := logger.Ctx(cmd.Context()) + ctx := cmd.Context() + log := logger.Ctx(ctx) + + if utils.HasNoFlagsAndShownHelp(cmd) { + return nil + } overrides := map[string]string{ credentials.AWSAccessKeyID: accessKey, @@ -147,11 +157,11 @@ func connectS3Cmd(cmd *cobra.Command, args []string) error { "accessKey", s3Cfg.AccessKey, "hasSecretKey", len(s3Cfg.SecretKey) > 0) - r, err := repository.Connect(cmd.Context(), a, s) + r, err := repository.Connect(ctx, a, s) if err != nil { return errors.Wrap(err, "Failed to connect to the S3 repository") } - defer utils.CloseRepo(cmd.Context(), r) + defer utils.CloseRepo(ctx, r) fmt.Printf("Connected to S3 bucket %s.\n", s3Cfg.Bucket) diff --git a/src/cli/restore/exchange.go b/src/cli/restore/exchange.go index 01bee2ff0..fa6132405 100644 --- a/src/cli/restore/exchange.go +++ b/src/cli/restore/exchange.go @@ -9,6 +9,7 @@ import ( "github.com/alcionai/corso/cli/config" "github.com/alcionai/corso/cli/utils" "github.com/alcionai/corso/pkg/credentials" + "github.com/alcionai/corso/pkg/logger" "github.com/alcionai/corso/pkg/repository" ) @@ -41,8 +42,9 @@ var exchangeCmd = &cobra.Command{ // initializes a s3 repo. func createExchangeCmd(cmd *cobra.Command, args []string) error { - if cmd.Flags().NFlag() == 0 { - cmd.Help() + ctx := cmd.Context() + + if utils.HasNoFlagsAndShownHelp(cmd) { return nil } @@ -65,25 +67,24 @@ func createExchangeCmd(cmd *cobra.Command, args []string) error { a.TenantID = cfgTenantID } - fmt.Printf( - "Called - %s\n\t365TenantID:\t%s\n\t356Client:\t%s\n\tfound 356Secret:\t%v\n", - cmd.CommandPath(), - m365.TenantID, - m365.ClientID, - len(m365.ClientSecret) > 0) + logger.Ctx(ctx).Debugw( + "Called - "+cmd.CommandPath(), + "tenantID", m365.TenantID, + "clientID", m365.ClientID, + "hasClientSecret", len(m365.ClientSecret) > 0) - r, err := repository.Connect(cmd.Context(), a, s) + r, err := repository.Connect(ctx, a, s) if err != nil { return errors.Wrapf(err, "Failed to connect to the %s repository", s.Provider) } - defer utils.CloseRepo(cmd.Context(), r) + defer utils.CloseRepo(ctx, r) - ro, err := r.NewRestore(cmd.Context(), []string{user, folder, mail}) + ro, err := r.NewRestore(ctx, []string{user, folder, mail}) if err != nil { return errors.Wrap(err, "Failed to initialize Exchange restore") } - if _, err := ro.Run(cmd.Context()); err != nil { + if _, err := ro.Run(ctx); err != nil { return errors.Wrap(err, "Failed to run Exchange restore") } diff --git a/src/cli/utils/utils.go b/src/cli/utils/utils.go index 8db3cb1c3..51f3b3bb8 100644 --- a/src/cli/utils/utils.go +++ b/src/cli/utils/utils.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/alcionai/corso/pkg/repository" + "github.com/spf13/cobra" ) // RequireProps validates the existence of the properties @@ -25,3 +26,16 @@ func CloseRepo(ctx context.Context, r *repository.Repository) { fmt.Print("Error closing repository:", err) } } + +// HasNoFlagsAndShownHelp shows the Help output if no flags +// were provided to the command. Returns true if the help +// was shown. +// Use for when the non-flagged usage of a command +// (ex: corso backup restore exchange) is expected to no-op. +func HasNoFlagsAndShownHelp(cmd *cobra.Command) bool { + if cmd.Flags().NFlag() == 0 { + cmd.Help() + return true + } + return false +} diff --git a/src/internal/connector/errors.go b/src/internal/connector/errors.go index da5305fc4..4f9967796 100644 --- a/src/internal/connector/errors.go +++ b/src/internal/connector/errors.go @@ -1,13 +1,15 @@ package connector import ( + "context" "fmt" "strings" - "github.com/pkg/errors" - multierror "github.com/hashicorp/go-multierror" msgraph_errors "github.com/microsoftgraph/msgraph-sdk-go/models/odataerrors" + "github.com/pkg/errors" + + "github.com/alcionai/corso/pkg/logger" ) // WrapErrorAndAppend helper function used to attach identifying information to an error @@ -24,10 +26,10 @@ func WrapAndAppendf(identifier interface{}, e error, previous error) error { // ListErrors is a helper method used to return the string of errors when // the multiError library is used. // depends on ConnectorStackErrorTrace -func ListErrors(multi multierror.Error) string { +func ListErrors(ctx context.Context, multi multierror.Error) string { aString := "" for idx, err := range multi.Errors { - detail := ConnectorStackErrorTrace(err) + detail := ConnectorStackErrorTrace(ctx, err) if detail == "" { detail = fmt.Sprintf("%v", err) } @@ -49,7 +51,7 @@ func concatenateStringFromPointers(orig string, pointers []*string) string { // ConnectorStackErrorTrace is a helper function that wraps the // stack trace for oDataError types from querying the M365 back store. -func ConnectorStackErrorTrace(e error) string { +func ConnectorStackErrorTrace(ctx context.Context, e error) string { eMessage := "" if oDataError, ok := e.(msgraph_errors.ODataErrorable); ok { // Get MainError @@ -81,7 +83,7 @@ func ConnectorStackErrorTrace(e error) string { } } if inners != nil { - fmt.Println("Inners not nil") + logger.Ctx(ctx).Debug("error contains inner errors") eMessage = eMessage + "\nConnector Section:" client := inners.GetClientRequestId() rId := inners.GetRequestId() diff --git a/src/internal/connector/errors_test.go b/src/internal/connector/errors_test.go index 8214572ee..d04108d97 100644 --- a/src/internal/connector/errors_test.go +++ b/src/internal/connector/errors_test.go @@ -1,6 +1,7 @@ package connector import ( + "context" "errors" "fmt" "strings" @@ -26,8 +27,8 @@ func (suite *GraphConnectorErrorSuite) TestWrapAndAppend() { suite.True(strings.Contains(returnErr.Error(), "arc376")) suite.Error(returnErr) multi := &multierror.Error{Errors: []error{err1, err2}} - suite.True(strings.Contains(ListErrors(*multi), "two")) // Does not contain the wrapped information - suite.T().Log(ListErrors(*multi)) + suite.True(strings.Contains(ListErrors(context.Background(), *multi), "two")) // Does not contain the wrapped information + suite.T().Log(ListErrors(context.Background(), *multi)) } func (suite *GraphConnectorErrorSuite) TestWrapAndAppend_OnVar() { diff --git a/src/internal/connector/graph_connector.go b/src/internal/connector/graph_connector.go index d78a929a5..3dc792e14 100644 --- a/src/internal/connector/graph_connector.go +++ b/src/internal/connector/graph_connector.go @@ -4,6 +4,7 @@ package connector import ( "bytes" + "context" "fmt" "io" @@ -17,6 +18,8 @@ import ( msuser "github.com/microsoftgraph/msgraph-sdk-go/users" msfolder "github.com/microsoftgraph/msgraph-sdk-go/users/item/mailfolders" "github.com/pkg/errors" + + "github.com/alcionai/corso/pkg/logger" ) const ( @@ -130,11 +133,11 @@ func buildFromMap(isKey bool, mapping map[string]string) []string { // Assumption: User exists // TODO: https://github.com/alcionai/corso/issues/135 // Add iota to this call -> mail, contacts, calendar, etc. -func (gc *GraphConnector) ExchangeDataCollection(user string) ([]DataCollection, error) { +func (gc *GraphConnector) ExchangeDataCollection(ctx context.Context, user string) ([]DataCollection, error) { // TODO replace with completion of Issue 124: //TODO: Retry handler to convert return: (DataCollection, error) - return gc.serializeMessages(user) + return gc.serializeMessages(ctx, user) } // optionsForMailFolders creates transforms the 'select' into a more dynamic call for MailFolders. @@ -154,7 +157,7 @@ func optionsForMailFolders(moreOps []string) *msfolder.MailFoldersRequestBuilder // restoreMessages: Utility function to connect to M365 backstore // and upload messages from DataCollection. // FullPath: tenantId, userId, , FolderId -func (gc *GraphConnector) restoreMessages(dc DataCollection) error { +func (gc *GraphConnector) restoreMessages(ctx context.Context, dc DataCollection) error { var errs error // must be user.GetId(), PrimaryName no longer works 6-15-2022 user := dc.FullPath()[1] @@ -188,7 +191,7 @@ func (gc *GraphConnector) restoreMessages(dc DataCollection) error { clone.SetIsDraft(&draft) sentMessage, err := gc.client.UsersById(user).MailFoldersById(address).Messages().Post(clone) if err != nil { - details := ConnectorStackErrorTrace(err) + details := ConnectorStackErrorTrace(ctx, err) errs = WrapAndAppend(data.UUID()+": "+details, err, errs) continue // TODO: Add to retry Handler for the for failure @@ -206,7 +209,7 @@ func (gc *GraphConnector) restoreMessages(dc DataCollection) error { // serializeMessages: Temp Function as place Holder until Collections have been added // to the GraphConnector struct. -func (gc *GraphConnector) serializeMessages(user string) ([]DataCollection, error) { +func (gc *GraphConnector) serializeMessages(ctx context.Context, user string) ([]DataCollection, error) { options := optionsForMailFolders([]string{}) response, err := gc.client.UsersById(user).MailFolders().GetWithRequestConfigurationAndResponseHandler(options, nil) if err != nil { @@ -258,7 +261,7 @@ func (gc *GraphConnector) serializeMessages(user string) ([]DataCollection, erro } } if err != nil { - fmt.Println("Retries exceeded") + logger.Ctx(ctx).Debug("exceeded maximum retries") errs = WrapAndAppend(*message.GetId(), fmt.Errorf("attachment failed: %v ", err), errs) } } @@ -286,7 +289,7 @@ func (gc *GraphConnector) serializeMessages(user string) ([]DataCollection, erro // Todo Retry Handler to be implemented edc.FinishPopulation() - //fmt.Printf("Storing ExchangeDataColection with %d items\n", edc.Length()) + logger.Ctx(ctx).Debugw("finished storing ExchangeDataColection", "itemCount", edc.Length()) collections = append(collections, &edc) } return collections, errs diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index a8f27b452..dbf520ba5 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -1,6 +1,7 @@ package connector import ( + "context" "testing" "github.com/stretchr/testify/assert" @@ -63,7 +64,7 @@ func (suite *GraphConnectorIntegrationSuite) TestGraphConnector_ExchangeDataColl if err := ctesting.RunOnAny(ctesting.CorsoCITests); err != nil { suite.T().Skip(err) } - collectionList, err := suite.connector.ExchangeDataCollection("lidiah@8qzvrj.onmicrosoft.com") + collectionList, err := suite.connector.ExchangeDataCollection(context.Background(), "lidiah@8qzvrj.onmicrosoft.com") assert.NotNil(suite.T(), collectionList) assert.Error(suite.T(), err) // TODO Remove after https://github.com/alcionai/corso/issues/140 if err != nil { @@ -89,7 +90,7 @@ func (suite *GraphConnectorIntegrationSuite) TestGraphConnector_restoreMessages( edc := NewExchangeDataCollection("tenant", []string{"tenantId", evs[user], mailCategory, "Inbox"}) edc.PopulateCollection(ds) edc.FinishPopulation() - err = suite.connector.restoreMessages(&edc) + err = suite.connector.restoreMessages(context.Background(), &edc) assert.NoError(suite.T(), err) } diff --git a/src/internal/operations/backup.go b/src/internal/operations/backup.go index 71c442ef0..5b488ff2a 100644 --- a/src/internal/operations/backup.go +++ b/src/internal/operations/backup.go @@ -55,7 +55,7 @@ func (op *BackupOperation) Run(ctx context.Context) (*kopia.BackupStats, error) return nil, errors.Wrap(err, "connecting to graph api") } - cs, err := gc.ExchangeDataCollection(op.Targets[0]) + cs, err := gc.ExchangeDataCollection(ctx, op.Targets[0]) if err != nil { return nil, errors.Wrap(err, "retrieving application data") }