Skip to content
This repository was archived by the owner on Jul 25, 2020. It is now read-only.

Conversation

@timuralp
Copy link
Contributor

@timuralp timuralp commented Mar 4, 2020

If there is no extended attribute support in the file system, the blobs
will not have their associated ETags available. In that case, the file
system blob store should rehash the content while producing the combined
blob and return the expected S3-style ETag.

blobs.add(blobPart);
md5Hasher.putBytes(BaseEncoding.base16().lowerCase().decode(blobPart.getMetadata().getETag()));
if (blobPart.getMetadata().getETag() != null) {
md5Hasher.putBytes(BaseEncoding.base16().lowerCase().decode(blobPart.getMetadata().getETag()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable, but did you consider returning an empty ETag instead? I appreciate the commitment to S3 fidelity buy FilesystemStorageStrategyImpl.getBlob already returns null ETags instead of recalculating them. Systems without extended attributes are uncommon so is the additional complexity worth it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did consider that and thought that previously the blob store returned the hash of the entire content on PUT. Looking back, it looks like it actually returned the hash of an empty string (as the blob part ETag didn't exist). I'll change this patch to the same behavior, keeping the -{number of parts} suffix for consistency.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might also want to look at listMultipartUpload. Recalculating ETags can be expensive if a client calls this in some kind of loop!

If there is no extended attribute support in the file system, the blobs
will not have their associated ETags available. In that case, the file
system blob store should rehash the content while producing the combined
blob and return the expected S3-style ETag.
@timuralp
Copy link
Contributor Author

timuralp commented Apr 3, 2020

Updated to return an empty-string MD5 if the ETag attribute of a blob is null.

@gaul gaul merged commit 7af4d8e into jclouds:master Apr 4, 2020
@gaul
Copy link
Member

gaul commented Apr 4, 2020

Sorry this should have been opened against apache/jclouds repository, not jclouds/jclouds. I pushed this to the correct master as 8d0e9dc and 2.2.x as e66bffa52168fa1c005b84477d9a4754c7a57074.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants