Skip to content

Commit 5037fcf

Browse files
committed
cleanup: return NotFound if blob stored w/o chunking
1 parent 9d079cf commit 5037fcf

File tree

3 files changed

+1
-31
lines changed

3 files changed

+1
-31
lines changed

server/remote_cache/chunking/chunking.go

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ package chunking
44
import (
55
"context"
66
"encoding/hex"
7-
"errors"
87
"flag"
98
"fmt"
109
"io"
@@ -293,17 +292,7 @@ func LoadManifest(ctx context.Context, cache interfaces.Cache, blobDigest *repb.
293292
if status.IsInternalError(err) {
294293
log.CtxInfof(ctx, "Failed to get chunking manifest (blob %s, manifest key %s): %v", blobDigest.GetHash(), acRNProto.GetDigest().GetHash(), err)
295294
}
296-
err = sanitizeManifestError(err, acRNProto.GetDigest().GetHash(), blobDigest.GetHash())
297-
if status.IsNotFoundError(err) {
298-
blobRN := digest.NewCASResourceName(blobDigest, instanceName, digestFunction).ToProto()
299-
if exists, existsErr := cache.Contains(ctx, blobRN); existsErr != nil {
300-
return nil, errors.Join(existsErr, err)
301-
} else if exists {
302-
return nil, status.UnimplementedErrorf("blob %s exists but was not stored with chunking", blobDigest.GetHash())
303-
}
304-
return nil, err
305-
}
306-
return nil, status.InternalErrorf("retrieve chunked manifest from AC: %w", err)
295+
return nil, sanitizeManifestError(err, acRNProto.GetDigest().GetHash(), blobDigest.GetHash())
307296
}
308297

309298
ar := &repb.ActionResult{}

server/remote_cache/chunking/chunking_test.go

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -183,22 +183,6 @@ func TestStoreAndLoad(t *testing.T) {
183183
}
184184
}
185185

186-
func TestLoadWithoutManifest_BlobExists(t *testing.T) {
187-
ctx := context.Background()
188-
te := testenv.GetTestEnv(t)
189-
ctx, err := prefix.AttachUserPrefixToContext(ctx, te.GetAuthenticator())
190-
require.NoError(t, err)
191-
cache := te.GetCache()
192-
193-
blobRN, blobData := testdigest.RandomCASResourceBuf(t, 500)
194-
require.NoError(t, cache.Set(ctx, blobRN, blobData))
195-
196-
_, err = chunking.LoadManifest(ctx, cache, blobRN.GetDigest(), "", repb.DigestFunction_SHA256)
197-
require.Error(t, err)
198-
require.True(t, status.IsUnimplementedError(err))
199-
assert.Contains(t, err.Error(), "exists but was not stored with chunking")
200-
}
201-
202186
func TestLoadWithoutManifest_BlobMissing(t *testing.T) {
203187
ctx := context.Background()
204188
te := testenv.GetTestEnv(t)

server/remote_cache/content_addressable_storage_server/content_addressable_storage_server.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1219,9 +1219,6 @@ func (s *ContentAddressableStorageServer) SplitBlob(ctx context.Context, req *re
12191219

12201220
manifest, err := chunking.LoadManifest(ctx, s.cache, req.GetBlobDigest(), req.GetInstanceName(), req.GetDigestFunction())
12211221
if err != nil {
1222-
// TODO(buildbuddy-internal#6426): Unimplemented is returned when the blob
1223-
// exists but wasn't stored with chunking. In the future, we could return
1224-
// the blob as a single chunk to better comply with the RE API contract.
12251222
return nil, err
12261223
}
12271224

0 commit comments

Comments
 (0)