From 040a541c74913c01ee3517273a7be30c510cc620 Mon Sep 17 00:00:00 2001 From: radhika Date: Wed, 25 Nov 2015 17:16:27 -0500 Subject: [PATCH] 7252: convert fatalf's into errors in logger sdk code; remove AssertFromString from blockdigest and instead use FromString in all places where it was being used. --- sdk/go/blockdigest/blockdigest.go | 10 ---------- sdk/go/blockdigest/blockdigest_test.go | 24 +++++++++++++++++++----- sdk/go/logger/logger.go | 16 +++++++++------- sdk/go/manifest/manifest_test.go | 25 +++++++++++++++++++------ services/datamanager/datamanager.go | 2 +- 5 files changed, 48 insertions(+), 29 deletions(-) diff --git a/sdk/go/blockdigest/blockdigest.go b/sdk/go/blockdigest/blockdigest.go index d2f1c60ba9..a5a668f940 100644 --- a/sdk/go/blockdigest/blockdigest.go +++ b/sdk/go/blockdigest/blockdigest.go @@ -3,7 +3,6 @@ package blockdigest import ( "fmt" - "log" "regexp" "strconv" "strings" @@ -58,15 +57,6 @@ func FromString(s string) (dig BlockDigest, err error) { return } -// Will fatal with the error if an error is encountered -func AssertFromString(s string) BlockDigest { - d, err := FromString(s) - if err != nil { - log.Fatalf("Error creating BlockDigest from %s: %v", s, err) - } - return d -} - func IsBlockLocator(s string) bool { return LocatorPattern.MatchString(s) } diff --git a/sdk/go/blockdigest/blockdigest_test.go b/sdk/go/blockdigest/blockdigest_test.go index 017aaa4710..e520deefe8 100644 --- a/sdk/go/blockdigest/blockdigest_test.go +++ b/sdk/go/blockdigest/blockdigest_test.go @@ -88,13 +88,20 @@ func TestInvalidDigestStrings(t *testing.T) { func TestBlockDigestWorksAsMapKey(t *testing.T) { m := make(map[BlockDigest]int) - bd := AssertFromString("01234567890123456789abcdefabcdef") + bd, err := FromString("01234567890123456789abcdefabcdef") + if err != nil { + t.Fatalf("Unexpected error during FromString for block: %v", err) + } m[bd] = 5 } func TestBlockDigestGetsPrettyPrintedByPrintf(t *testing.T) { input := "01234567890123456789abcdefabcdef" - prettyPrinted := fmt.Sprintf("%v", AssertFromString(input)) + fromString, err := FromString(input) + if err != nil { + t.Fatalf("Unexpected error during FromString: %v", err) + } + prettyPrinted := fmt.Sprintf("%v", fromString) if prettyPrinted != input { t.Fatalf("Expected blockDigest produced from \"%s\" to be printed as "+ "\"%s\", but instead it was printed as %s", @@ -103,7 +110,10 @@ func TestBlockDigestGetsPrettyPrintedByPrintf(t *testing.T) { } func TestBlockDigestGetsPrettyPrintedByPrintfInNestedStructs(t *testing.T) { - input := "01234567890123456789abcdefabcdef" + input, err := FromString("01234567890123456789abcdefabcdef") + if err != nil { + t.Fatalf("Unexpected error during FromString for block: %v", err) + } value := 42 nested := struct { // Fun trivia fact: If this field was called "digest" instead of @@ -113,7 +123,7 @@ func TestBlockDigestGetsPrettyPrintedByPrintfInNestedStructs(t *testing.T) { Digest BlockDigest value int }{ - AssertFromString(input), + input, value, } prettyPrinted := fmt.Sprintf("%+v", nested) @@ -155,7 +165,11 @@ func TestParseBlockLocatorSimple(t *testing.T) { if err != nil { t.Fatalf("Unexpected error parsing block locator: %v", err) } - expectBlockLocator(t, b, BlockLocator{Digest: AssertFromString("365f83f5f808896ec834c8b595288735"), + d, err := FromString("365f83f5f808896ec834c8b595288735") + if err != nil { + t.Fatalf("Unexpected error during FromString for block: %v", err) + } + expectBlockLocator(t, b, BlockLocator{Digest: d, Size: 2310, Hints: []string{"K@qr1hi", "Af0c9a66381f3b028677411926f0be1c6282fe67c@542b5ddf"}}) diff --git a/sdk/go/logger/logger.go b/sdk/go/logger/logger.go index a989afcf26..3b2db3a321 100644 --- a/sdk/go/logger/logger.go +++ b/sdk/go/logger/logger.go @@ -23,6 +23,7 @@ package logger import ( + "fmt" "git.curoverse.com/arvados.git/sdk/go/arvadosclient" "log" "time" @@ -73,16 +74,18 @@ type Logger struct { } // Create a new logger based on the specified parameters. -func NewLogger(params LoggerParams) *Logger { +func NewLogger(params LoggerParams) (l *Logger, err error) { // sanity check parameters if ¶ms.Client == nil { - log.Fatal("Nil arvados client in LoggerParams passed in to NewLogger()") + err = fmt.Errorf("Nil arvados client in LoggerParams passed in to NewLogger()") + return } if params.EventTypePrefix == "" { - log.Fatal("Empty event type prefix in LoggerParams passed in to NewLogger()") + err = fmt.Errorf("Empty event type prefix in LoggerParams passed in to NewLogger()") + return } - l := &Logger{ + l = &Logger{ data: make(map[string]interface{}), entry: make(map[string]interface{}), properties: make(map[string]interface{}), @@ -97,7 +100,7 @@ func NewLogger(params LoggerParams) *Logger { // Start the worker goroutine. go l.work() - return l + return l, nil } // Exported functions will be called from other goroutines, therefore @@ -196,7 +199,6 @@ func (l *Logger) write(isFinal bool) { // client. err := l.params.Client.Create("logs", l.data, nil) if err != nil { - log.Printf("Attempted to log: %v", l.data) - log.Fatalf("Received error writing log: %v", err) + log.Printf("Received error writing %v: %v", l.data, err) } } diff --git a/sdk/go/manifest/manifest_test.go b/sdk/go/manifest/manifest_test.go index 364648d643..a7ed833ae8 100644 --- a/sdk/go/manifest/manifest_test.go +++ b/sdk/go/manifest/manifest_test.go @@ -84,10 +84,15 @@ func TestParseBlockLocatorSimple(t *testing.T) { if err != nil { t.Fatalf("Unexpected error parsing block locator: %v", err) } - expectBlockLocator(t, b, BlockLocator{Digest: blockdigest.AssertFromString("365f83f5f808896ec834c8b595288735"), - Size: 2310, - Hints: []string{"K@qr1hi", - "Af0c9a66381f3b028677411926f0be1c6282fe67c@542b5ddf"}}) + d, err := blockdigest.FromString("365f83f5f808896ec834c8b595288735") + if err != nil { + t.Fatalf("Unexpected error during FromString for block locator: %v", err) + } + expectBlockLocator(t, blockdigest.BlockLocator{b.Digest, b.Size, b.Hints}, + blockdigest.BlockLocator{Digest: d, + Size: 2310, + Hints: []string{"K@qr1hi", + "Af0c9a66381f3b028677411926f0be1c6282fe67c@542b5ddf"}}) } func TestStreamIterShortManifestWithBlankStreams(t *testing.T) { @@ -121,9 +126,13 @@ func TestBlockIterLongManifest(t *testing.T) { blockChannel := manifest.BlockIterWithDuplicates() firstBlock := <-blockChannel + d, err := blockdigest.FromString("b746e3d2104645f2f64cd3cc69dd895d") + if err != nil { + t.Fatalf("Unexpected error during FromString for block: %v", err) + } expectBlockLocator(t, firstBlock, - blockdigest.BlockLocator{Digest: blockdigest.AssertFromString("b746e3d2104645f2f64cd3cc69dd895d"), + blockdigest.BlockLocator{Digest: d, Size: 15693477, Hints: []string{"E2866e643690156651c03d876e638e674dcd79475@5441920c"}}) blocksRead := 1 @@ -134,9 +143,13 @@ func TestBlockIterLongManifest(t *testing.T) { } expectEqual(t, blocksRead, 853) + d, err = blockdigest.FromString("f9ce82f59e5908d2d70e18df9679b469") + if err != nil { + t.Fatalf("Unexpected error during FromString for block: %v", err) + } expectBlockLocator(t, lastBlock, - blockdigest.BlockLocator{Digest: blockdigest.AssertFromString("f9ce82f59e5908d2d70e18df9679b469"), + blockdigest.BlockLocator{Digest: d, Size: 31367794, Hints: []string{"E53f903684239bcc114f7bf8ff9bd6089f33058db@5441920c"}}) } diff --git a/services/datamanager/datamanager.go b/services/datamanager/datamanager.go index 3c53b4ad2a..27011fa7ae 100644 --- a/services/datamanager/datamanager.go +++ b/services/datamanager/datamanager.go @@ -79,7 +79,7 @@ func singlerun(arv arvadosclient.ArvadosClient) error { } if logEventTypePrefix != "" { - arvLogger = logger.NewLogger(logger.LoggerParams{ + arvLogger, err = logger.NewLogger(logger.LoggerParams{ Client: arv, EventTypePrefix: logEventTypePrefix, WriteInterval: time.Second * time.Duration(logFrequencySeconds)}) -- 2.30.2