7252: convert fatalf's into errors in logger sdk code; remove AssertFromString from...

diff --git a/sdk/go/blockdigest/blockdigest.go b/sdk/go/blockdigest/blockdigest.go
index d2f1c60ba93889614de18756dcf3ed6608f0b5fa..a5a668f940f37b18418a29d28d02019e938f7a79 100644 (file)
--- 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 017aaa47101c9a2e1971e7694abac24bc77f1e06..e520deefe85315d14294e7bcce8683d8eaf6a299 100644 (file)
--- 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 a989afcf26cb7a6d7b7bad6e4b1589d30e06520d..3b2db3a321271495a3965759363e2d723ea9e082 100644 (file)
--- 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 &params.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 364648d643cea64dadc706eb1f5c42e227ccbb39..a7ed833ae8708b66bbc2b20c07bc8ce9936fd302 100644 (file)
--- 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 3c53b4ad2a2c59e6cea0b538ff1b7ca6f5999831..27011fa7ae3276568d2477f3f109419610cefff3 100644 (file)
--- 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)})