fix: update GrpcConversions to use Bucket.RetentionPolicy.retention_duration#1798
fix: update GrpcConversions to use Bucket.RetentionPolicy.retention_duration#1798BenWhitehead merged 2 commits intomainfrom
Conversation
e5f06a8 to
7d0570a
Compare
7d0570a to
f074a70
Compare
…uration Also update Apiary Conversions to carry through all fields, while updating HttpStorageRpc#update(Bucket) to take rpc specific action to filter down fields in the case of a patch.
f074a70 to
0bf09a4
Compare
| bucket.setRetentionPolicy(Data.nullOf(RetentionPolicy.class)); | ||
| } else { | ||
| retentionPolicy.setEffectiveTime(Data.nullOf(DateTime.class)); | ||
| retentionPolicy.setIsLocked(Data.nullOf(Boolean.class)); |
There was a problem hiding this comment.
Why do you set these values to the "null of" of their type? This does seem necessary in the client if they're read-only anyway.
There was a problem hiding this comment.
That else branch is leftover from some debugging, I'll clean it up. You're correct, it effectively don't have any impact.
| private static void maybeEncodeRetentionPolicy(BucketInfo from, Bucket to) { | ||
| if (from.getRetentionPeriodDuration() != null | ||
| || from.retentionPolicyIsLocked() != null | ||
| || from.getRetentionEffectiveTimeOffsetDateTime() != null) { |
There was a problem hiding this comment.
Why not use from.getRententionPeriodDuration() or from.retentionPolicyIsLocked() only and leave out from.getRetentionEffectiveTimeOffsetDateTime()?
There was a problem hiding this comment.
These converters are ignorant to the services constraints on fields. Our converters are always fully mapping, so we care about each of the fields. When this makes its way down to an actual request, that request handler is the place to apply service level field filtering.
No description provided.