[ARVADOS] updated: 779e41aaaa1f4d3a696a63011c00648bfbfaae72

git at public.curoverse.com git at public.curoverse.com
Thu Nov 26 00:22:51 EST 2015


Summary of changes:
 sdk/go/manifest/manifest.go                   | 21 +++++++++------------
 sdk/go/manifest/manifest_test.go              | 23 ++++++++++-------------
 sdk/python/arvados/_ranges.py                 |  4 ++++
 sdk/python/arvados/arvfile.py                 |  3 ---
 sdk/python/arvados/keep.py                    |  2 ++
 services/datamanager/collection/collection.go | 19 ++++++++++---------
 6 files changed, 35 insertions(+), 37 deletions(-)

       via  779e41aaaa1f4d3a696a63011c00648bfbfaae72 (commit)
       via  0c0f18dfbcdcf552889258b76563315fbe2eb060 (commit)
       via  f486405f195083289f543b5524c4059b01f49026 (commit)
       via  9045a0861e96a7c9f2717e9c5c34760cb3a30f66 (commit)
      from  11353b25e92155e86a2f1a5d7f9d7462fb8aa8a0 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 779e41aaaa1f4d3a696a63011c00648bfbfaae72
Author: radhika <radhika at curoverse.com>
Date:   Thu Nov 26 00:22:12 2015 -0500

    7253: update BlockIterWithDuplicates to return any errors through Manifest, rather than BlockLocator.

diff --git a/sdk/go/manifest/manifest.go b/sdk/go/manifest/manifest.go
index 49faa01..6deb88f 100644
--- a/sdk/go/manifest/manifest.go
+++ b/sdk/go/manifest/manifest.go
@@ -20,6 +20,7 @@ var LocatorPattern = regexp.MustCompile(
 
 type Manifest struct {
 	Text string
+	Err  error
 }
 
 type BlockLocator struct {
@@ -246,28 +247,24 @@ func (m *Manifest) FileSegmentIterByName(filepath string) <-chan *FileSegment {
 	return ch
 }
 
-type ManifestBlockLocator struct {
-	Locator blockdigest.BlockLocator
-	Err     error
-}
-
 // Blocks may appear mulitple times within the same manifest if they
 // are used by multiple files. In that case this Iterator will output
 // the same block multiple times.
-func (m *Manifest) BlockIterWithDuplicates() <-chan ManifestBlockLocator {
-	blockChannel := make(chan ManifestBlockLocator)
+//
+// In order to detect parse errors, caller must check m.Err after the returned channel closes.
+func (m *Manifest) BlockIterWithDuplicates() <-chan blockdigest.BlockLocator {
+	blockChannel := make(chan blockdigest.BlockLocator)
 	go func(streamChannel <-chan ManifestStream) {
 		for ms := range streamChannel {
 			if ms.Err != nil {
-				blockChannel <- ManifestBlockLocator{Locator: blockdigest.BlockLocator{}, Err: ms.Err}
+				m.Err = ms.Err
 				continue
 			}
 			for _, block := range ms.Blocks {
-				b, err := blockdigest.ParseBlockLocator(block)
-				if err == nil {
-					blockChannel <- ManifestBlockLocator{b, nil}
+				if b, err := blockdigest.ParseBlockLocator(block); err == nil {
+					blockChannel <- b
 				} else {
-					blockChannel <- ManifestBlockLocator{Locator: blockdigest.BlockLocator{}, Err: err}
+					m.Err = err
 				}
 			}
 		}
diff --git a/sdk/go/manifest/manifest_test.go b/sdk/go/manifest/manifest_test.go
index 65ea7f1..53d4a15 100644
--- a/sdk/go/manifest/manifest_test.go
+++ b/sdk/go/manifest/manifest_test.go
@@ -96,7 +96,7 @@ func TestStreamIterShortManifestWithBlankStreams(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Unexpected error reading manifest from file: %v", err)
 	}
-	manifest := Manifest{string(content)}
+	manifest := Manifest{Text: string(content)}
 	streamIter := manifest.StreamIter()
 
 	firstStream := <-streamIter
@@ -118,25 +118,25 @@ func TestBlockIterLongManifest(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Unexpected error reading manifest from file: %v", err)
 	}
-	manifest := Manifest{string(content)}
+	manifest := Manifest{Text: string(content)}
 	blockChannel := manifest.BlockIterWithDuplicates()
 
 	firstBlock := <-blockChannel
 
 	expectBlockLocator(t,
-		firstBlock.Locator,
+		firstBlock,
 		blockdigest.BlockLocator{Digest: blockdigest.AssertFromString("b746e3d2104645f2f64cd3cc69dd895d"),
 			Size:  15693477,
 			Hints: []string{"E2866e643690156651c03d876e638e674dcd79475 at 5441920c"}})
 	blocksRead := 1
-	var lastBlock ManifestBlockLocator
+	var lastBlock blockdigest.BlockLocator
 	for lastBlock = range blockChannel {
 		blocksRead++
 	}
 	expectEqual(t, blocksRead, 853)
 
 	expectBlockLocator(t,
-		lastBlock.Locator,
+		lastBlock,
 		blockdigest.BlockLocator{Digest: blockdigest.AssertFromString("f9ce82f59e5908d2d70e18df9679b469"),
 			Size:  31367794,
 			Hints: []string{"E53f903684239bcc114f7bf8ff9bd6089f33058db at 5441920c"}})
@@ -211,24 +211,21 @@ func TestBlockIterWithBadManifest(t *testing.T) {
 	}
 
 	for _, testCase := range testCases {
-		manifest := Manifest{string(testCase[0])}
+		manifest := Manifest{Text: string(testCase[0])}
 		blockChannel := manifest.BlockIterWithDuplicates()
 
-		var err error
 		for block := range blockChannel {
-			if block.Err != nil {
-				err = block.Err
-			}
+			_ = block
 		}
 
 		// completed reading from blockChannel; now check for errors
-		if err == nil {
+		if manifest.Err == nil {
 			t.Errorf("Expected error")
 		}
 
-		matched, _ := regexp.MatchString(testCase[1], err.Error())
+		matched, _ := regexp.MatchString(testCase[1], manifest.Err.Error())
 		if !matched {
-			t.Errorf("Expected error not found. Expected: %v; Found: %v", testCase[1], err.Error())
+			t.Errorf("Expected error not found. Expected: %v; Found: %v", testCase[1], manifest.Err.Error())
 		}
 	}
 }
diff --git a/services/datamanager/collection/collection.go b/services/datamanager/collection/collection.go
index 0d583c2..df68526 100644
--- a/services/datamanager/collection/collection.go
+++ b/services/datamanager/collection/collection.go
@@ -274,7 +274,7 @@ func ProcessCollections(arvLogger *logger.Logger,
 			collection.ReplicationLevel = defaultReplicationLevel
 		}
 
-		manifest := manifest.Manifest{sdkCollection.ManifestText}
+		manifest := manifest.Manifest{Text: sdkCollection.ManifestText}
 		manifestSize := uint64(len(sdkCollection.ManifestText))
 
 		if _, alreadySeen := UUIDToCollection[collection.UUID]; !alreadySeen {
@@ -286,21 +286,22 @@ func ProcessCollections(arvLogger *logger.Logger,
 
 		blockChannel := manifest.BlockIterWithDuplicates()
 		for block := range blockChannel {
-			if block.Err != nil {
-				err = block.Err
-				return
-			}
-			if storedSize, stored := collection.BlockDigestToSize[block.Locator.Digest]; stored && storedSize != block.Locator.Size {
+			if storedSize, stored := collection.BlockDigestToSize[block.Digest]; stored && storedSize != block.Size {
 				err = fmt.Errorf(
 					"Collection %s contains multiple sizes (%d and %d) for block %s",
 					collection.UUID,
 					storedSize,
-					block.Locator.Size,
-					block.Locator.Digest)
+					block.Size,
+					block.Digest)
 				return
 			}
-			collection.BlockDigestToSize[block.Locator.Digest] = block.Locator.Size
+			collection.BlockDigestToSize[block.Digest] = block.Size
 		}
+		if manifest.Err != nil {
+			err = manifest.Err
+			return
+		}
+
 		collection.TotalSize = 0
 		for _, size := range collection.BlockDigestToSize {
 			collection.TotalSize += size

commit 0c0f18dfbcdcf552889258b76563315fbe2eb060
Merge: 11353b2 f486405
Author: radhika <radhika at curoverse.com>
Date:   Wed Nov 25 22:52:29 2015 -0500

    Merge branch 'master' into 7253-datamanager-test-errors


-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list