[ARVADOS] updated: 7b5798a7ab66c40c4a3feb028fc06478f40bd425
Git user
git at public.curoverse.com
Fri May 6 14:37:14 EDT 2016
Summary of changes:
services/keepstore/azure_blob_volume.go | 54 ++++++++++++++++++++-----------
services/keepstore/volume_generic_test.go | 32 +++++++++---------
2 files changed, 51 insertions(+), 35 deletions(-)
via 7b5798a7ab66c40c4a3feb028fc06478f40bd425 (commit)
from b9d5f6444ca07c09d4d75ba3fb87271137e5f981 (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 7b5798a7ab66c40c4a3feb028fc06478f40bd425
Author: radhika <radhika at curoverse.com>
Date: Fri May 6 14:36:25 2016 -0400
8556: improve checkTrashed method signature
diff --git a/services/keepstore/azure_blob_volume.go b/services/keepstore/azure_blob_volume.go
index e111d96..b3811fc 100644
--- a/services/keepstore/azure_blob_volume.go
+++ b/services/keepstore/azure_blob_volume.go
@@ -135,15 +135,15 @@ func (v *AzureBlobVolume) Check() error {
}
// Return NotFoundError if trash marker is found on the block
-func (v *AzureBlobVolume) checkTrashed(loc string) error {
+func (v *AzureBlobVolume) checkTrashed(loc string) (bool, error) {
metadata, err := v.bsClient.GetBlobMetadata(v.containerName, loc)
if err != nil {
- return err
+ return false, v.translateError(err)
}
if metadata["expires_at"] != "" {
- return v.translateError(NotFoundError)
+ return true, v.translateError(NotFoundError)
}
- return nil
+ return false, nil
}
// Get reads a Keep block that has been stored as a block blob in the
@@ -153,9 +153,13 @@ func (v *AzureBlobVolume) checkTrashed(loc string) error {
// unexpectedly empty, assume a PutBlob operation is in progress, and
// wait for it to finish writing.
func (v *AzureBlobVolume) Get(loc string, buf []byte) (int, error) {
- if err := v.checkTrashed(loc); err != nil {
+ trashed, err := v.checkTrashed(loc)
+ if err != nil {
return 0, err
}
+ if trashed == true {
+ return 0, os.ErrNotExist
+ }
var deadline time.Time
haveDeadline := false
size, err := v.get(loc, buf)
@@ -257,9 +261,13 @@ func (v *AzureBlobVolume) get(loc string, buf []byte) (int, error) {
// Compare the given data with existing stored data.
func (v *AzureBlobVolume) Compare(loc string, expect []byte) error {
- if err := v.checkTrashed(loc); err != nil {
+ trashed, err := v.checkTrashed(loc)
+ if err != nil {
return err
}
+ if trashed == true {
+ return os.ErrNotExist
+ }
rdr, err := v.bsClient.GetBlob(v.containerName, loc)
if err != nil {
return v.translateError(err)
@@ -301,25 +309,33 @@ func (v *AzureBlobVolume) Touch(loc string) error {
if v.readonly {
return MethodDisabledError
}
- if err := v.checkTrashed(loc); err != nil {
+ trashed, err := v.checkTrashed(loc)
+ if err != nil {
return err
}
+ if trashed == true {
+ return os.ErrNotExist
+ }
return v.addToMetadata(loc, "last_write_at", fmt.Sprintf("%d", time.Now()))
}
// Mtime returns the last-modified property of a block blob.
func (v *AzureBlobVolume) Mtime(loc string) (time.Time, error) {
- if err := v.checkTrashed(loc); err != nil {
+ trashed, err := v.checkTrashed(loc)
+ if err != nil {
return time.Time{}, err
}
+ if trashed == true {
+ return time.Time{}, os.ErrNotExist
+ }
metadata, err := v.bsClient.GetBlobMetadata(v.containerName, loc)
if err != nil {
- return time.Time{}, err
+ return time.Time{}, v.translateError(err)
}
lastWriteAt, err := strconv.ParseInt(metadata["last_write_at"], 10, 64)
if err != nil {
- return time.Time{}, err
+ return time.Time{}, v.translateError(err)
}
return time.Unix(lastWriteAt, 0), nil
}
@@ -375,22 +391,22 @@ func (v *AzureBlobVolume) Trash(loc string) error {
// we get the Etag before checking Mtime, and use If-Match to
// ensure we don't delete data if Put() or Touch() happens
// between our calls to Mtime() and DeleteBlob().
+ props, err := v.bsClient.GetBlobProperties(v.containerName, loc)
+ if err != nil {
+ return err
+ }
if t, err := v.Mtime(loc); err != nil {
return err
} else if time.Since(t) < blobSignatureTTL {
return nil
}
if trashLifetime == 0 {
- props, err := v.bsClient.GetBlobProperties(v.containerName, loc)
- if err != nil {
- return err
- }
return v.bsClient.DeleteBlob(v.containerName, loc, map[string]string{
"If-Match": props.Etag,
})
}
// Mark as trash
- err := v.addToMetadata(loc, "expires_at", fmt.Sprintf("%d", time.Now().Add(trashLifetime).Unix()))
+ err = v.addToMetadata(loc, "expires_at", fmt.Sprintf("%d", time.Now().Add(trashLifetime).Unix()))
if err != nil {
return err
}
@@ -404,7 +420,7 @@ func (v *AzureBlobVolume) Untrash(loc string) error {
// if expires_at does not exist, return NotFoundError
metadata, err := v.bsClient.GetBlobMetadata(v.containerName, loc)
if err != nil {
- return err
+ return v.translateError(err)
}
if metadata["expires_at"] == "" {
return v.translateError(NotFoundError)
@@ -412,12 +428,12 @@ func (v *AzureBlobVolume) Untrash(loc string) error {
// reset expires_at metadata attribute
err = v.removeFromMetadata(loc, "expires_at")
if err != nil {
- return err
+ return v.translateError(err)
}
// delete trash marker if exists
_, err = v.bsClient.DeleteBlobIfExists(v.containerName, fmt.Sprintf("trash.%v.%v", metadata["expires_at"], loc), map[string]string{})
- return err
+ return v.translateError(err)
}
// Status returns a VolumeStatus struct with placeholder data.
@@ -452,7 +468,7 @@ func (v *AzureBlobVolume) translateError(err error) error {
switch {
case err == nil:
return err
- case strings.Contains(err.Error(), "404 Not Found"):
+ case strings.Contains(err.Error(), "Not Found"):
// "storage: service returned without a response body (404 Not Found)"
return os.ErrNotExist
default:
diff --git a/services/keepstore/volume_generic_test.go b/services/keepstore/volume_generic_test.go
index b31b236..a3321cc 100644
--- a/services/keepstore/volume_generic_test.go
+++ b/services/keepstore/volume_generic_test.go
@@ -120,7 +120,7 @@ func testCompareNonexistent(t TB, factory TestableVolumeFactory) {
defer v.Teardown()
err := v.Compare(TestHash, TestBlock)
- if err != os.ErrNotExist && !strings.Contains(err.Error(), "Not Found") {
+ if err == nil || !os.IsNotExist(err) {
t.Errorf("Got err %T %q, expected os.ErrNotExist", err, err)
}
}
@@ -451,17 +451,17 @@ func testDeleteOldBlock(t TB, factory TestableVolumeFactory) {
}
data := make([]byte, BlockSize)
_, err := v.Get(TestHash, data)
- if err == nil || (!os.IsNotExist(err) && !strings.Contains(err.Error(), "Not Found")) {
+ if err == nil || !os.IsNotExist(err) {
t.Errorf("os.IsNotExist(%v) should have been true", err)
}
_, err = v.Mtime(TestHash)
- if err == nil || (!os.IsNotExist(err) && !strings.Contains(err.Error(), "Not Found")) {
+ if err == nil {
t.Fatalf("os.IsNotExist(%v) should have been true", err)
}
err = v.Compare(TestHash, TestBlock)
- if err == nil || (!os.IsNotExist(err) && !strings.Contains(err.Error(), "Not Found")) {
+ if err == nil || !os.IsNotExist(err) {
t.Fatalf("os.IsNotExist(%v) should have been true", err)
}
@@ -472,7 +472,7 @@ func testDeleteOldBlock(t TB, factory TestableVolumeFactory) {
}
err = v.Touch(TestHash)
- if err == nil || (!os.IsNotExist(err) && !strings.Contains(err.Error(), "Not Found")) {
+ if err == nil || !os.IsNotExist(err) {
t.Fatalf("os.IsNotExist(%v) should have been true", err)
}
}
@@ -753,7 +753,7 @@ func testTrashUntrash(t TB, factory TestableVolumeFactory) {
}
} else {
_, err = v.Get(TestHash, buf)
- if err == nil || (!os.IsNotExist(err) && !strings.Contains(err.Error(), "Not Found")) {
+ if err == nil || !os.IsNotExist(err) {
t.Errorf("os.IsNotExist(%v) should have been true", err)
}
@@ -813,17 +813,17 @@ func testTrashEmptyTrashUntrash(t TB, factory TestableVolumeFactory) {
}
err = checkGet()
- if err == nil || (!os.IsNotExist(err) && !strings.Contains(err.Error(), "Not Found")) {
+ if err == nil || !os.IsNotExist(err) {
t.Fatalf("os.IsNotExist(%v) should have been true", err)
}
_, err = v.Mtime(TestHash)
- if err == nil || (!os.IsNotExist(err) && !strings.Contains(err.Error(), "Not Found")) {
+ if err == nil || !os.IsNotExist(err) {
t.Fatalf("os.IsNotExist(%v) should have been true", err)
}
err = v.Compare(TestHash, TestBlock)
- if err == nil || (!os.IsNotExist(err) && !strings.Contains(err.Error(), "Not Found")) {
+ if err == nil || !os.IsNotExist(err) {
t.Fatalf("os.IsNotExist(%v) should have been true", err)
}
@@ -834,7 +834,7 @@ func testTrashEmptyTrashUntrash(t TB, factory TestableVolumeFactory) {
}
err = v.Touch(TestHash)
- if err == nil || (!os.IsNotExist(err) && !strings.Contains(err.Error(), "Not Found")) {
+ if err == nil || !os.IsNotExist(err) {
t.Fatalf("os.IsNotExist(%v) should have been true", err)
}
@@ -879,7 +879,7 @@ func testTrashEmptyTrashUntrash(t TB, factory TestableVolumeFactory) {
// Untrash should fail if the only block in the trash has
// already been untrashed.
err = v.Untrash(TestHash)
- if err == nil || (!os.IsNotExist(err) && !strings.Contains(err.Error(), "Not Found")) {
+ if err == nil || !os.IsNotExist(err) {
t.Fatalf("os.IsNotExist(%v) should have been true", err)
}
@@ -899,7 +899,7 @@ func testTrashEmptyTrashUntrash(t TB, factory TestableVolumeFactory) {
t.Fatal(err)
}
err = checkGet()
- if err == nil || (!os.IsNotExist(err) && !strings.Contains(err.Error(), "Not Found")) {
+ if err == nil || !os.IsNotExist(err) {
t.Fatalf("os.IsNotExist(%v) should have been true", err)
}
@@ -918,20 +918,20 @@ func testTrashEmptyTrashUntrash(t TB, factory TestableVolumeFactory) {
// goes away.
err = v.Trash(TestHash)
err = checkGet()
- if err == nil || (!os.IsNotExist(err) && !strings.Contains(err.Error(), "Not Found")) {
+ if err == nil || !os.IsNotExist(err) {
t.Fatalf("os.IsNotExist(%v) should have been true", err)
}
v.EmptyTrash()
// Untrash won't find it
err = v.Untrash(TestHash)
- if err == nil || (!os.IsNotExist(err) && !strings.Contains(err.Error(), "Not Found")) {
+ if err == nil || !os.IsNotExist(err) {
t.Fatalf("os.IsNotExist(%v) should have been true", err)
}
// Get block won't find it
err = checkGet()
- if err == nil || (!os.IsNotExist(err) && !strings.Contains(err.Error(), "Not Found")) {
+ if err == nil || !os.IsNotExist(err) {
t.Fatalf("os.IsNotExist(%v) should have been true", err)
}
@@ -948,7 +948,7 @@ func testTrashEmptyTrashUntrash(t TB, factory TestableVolumeFactory) {
t.Fatal(err)
}
err = checkGet()
- if err == nil || (!os.IsNotExist(err) && !strings.Contains(err.Error(), "Not Found")) {
+ if err == nil || !os.IsNotExist(err) {
t.Fatalf("os.IsNotExist(%v) should have been true", err)
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list