Add replication for missing blobs in readFallbackBlobAccess #174
Conversation
The changes are very similar to mirroredBlobAccess
There was a problem hiding this comment.
Hey @annapst,
This change looks pretty good. Thanks for working on this. Be sure to process the comments I've left behind, and I'll merge this in a timely manner.
| @@ -90,5 +91,23 @@ func (ba *readFallbackBlobAccess) FindMissing(ctx context.Context, digests diges | |||
| if err != nil { | |||
| return digest.EmptySet, util.StatusWrap(err, "Secondary") | |||
| } | |||
|
|
|||
| // Replicate the blobs that are present only in the secondary backend to primary | |||
| if ba.replicator != nil { | |||
There was a problem hiding this comment.
This if-statement can be removed. The Buildbarn codebase tries to follow the null object pattern where possible. So if you specify a 'noop' blob replicator, it's actually an instance of this type:
There was a problem hiding this comment.
I've added this check because of the description of a previous pull request touching this file, mentioning backwards compatibility (47ea975) and because there is a nil check in other places in this file.
Is this still safe to remove with this info?
There was a problem hiding this comment.
It's still safe to remove it. What's in the commit message likely applied to the initial version of the PR. See this comment in the PR, where I requested the same thing:
There was a problem hiding this comment.
Thank you for clarifying! Let me update!
The changes are very similar to mirroredBlobAccess