add storage ID support to format/hash
#59
Conversation
| CPartLive // live content part hash --> including storage ID | ||
|
|
||
| // live content part hash including storage ID that doesn't generate a regular part upon finalization | ||
| // PENDING(LUK): is this really needed? |
There was a problem hiding this comment.
I think CPartLiveTransient could still be needed for the sake of live-to-vod perhaps. I could see having a storage_id specified would be useful in the case that we persist these (transient) live parts later, e.g. live-to-vod. Otherwise, we would update live-to-vod to accept a storage_id.
This brings up another question about live parts: since they all expire, would it be more useful for live parts to have two storage_ids? One for the expire-able live part, and one for the permanent part (either the hcp_ or for when persisted via live-to-vod)?
| case CPartLive: | ||
| c = "live content part with storage id" | ||
| case CPartLiveTransient: | ||
| c = "transient live content part" |
There was a problem hiding this comment.
"transient live content part with storage id"
|
|
||
| // NewObject creates a new object hash with the given type, digest, size, and ID | ||
| // | ||
| // Deprecated: use NewBuilder().BuildHash(), then AsContentHash() instead. |
There was a problem hiding this comment.
While I agree that "regular" code in general should not be using these constructors, I think they were initially only provided as helpers for tests that generate hashes for their use. I am inclined to still provide some kind of constructor for ease of generating hashes. Maybe go back to just a monolithic New constructor?
| } | ||
|
|
||
| // newPart creates a new non-live part hash with the given type, digest, size, preamble size and storage ID | ||
| func newPart(format Format, digest []byte, size uint64, preambleSize uint64, storageId uint) (*Hash, error) { |
There was a problem hiding this comment.
I think NewLive below also needs the storageId argument added
|
|
||
| //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| func CalcHash(reader io.ReadSeeker, size ...int64) (*Hash, error) { |
There was a problem hiding this comment.
Maybe add a comment and move the unit test from content-fabric in qfab/cmd/part/calc_hash_test.go that says:
// PENDING(JON): Move to common-go
func ExampleCalcHash() {
Adds new hash types that include a storage ID (C, CPart, CPartLive, CPartLiveTransient).
I deprecated a couple constructor functions because they are potentially unsafe and lead wrong utilization. Instead, I added a hash builder (implemented by the Digest, because that’s what it essentially is) and explicit conversion functions to convert hashes from part to content hashes and vice-versa. I did not remove any of the old functions because they are used all over the place in unit tests. An exception to this is Factory.NewContentDigest(), which makes really no sense, so I removed it.
This changeset requires few changes in content-fabric, branch
luk/hash-storage-id.