fix: update GrpcConversions to use Bucket.RetentionPolicy.retention_duration #1798
Conversation
product-auto-label
bot
added
size: m
BenWhitehead
force-pushed
the
grpc/policy-duration
branch
from
e5f06a8
to
7d0570a
Compare
BenWhitehead
force-pushed
the
grpc/policy-duration
branch
from
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.
BenWhitehead
force-pushed
the
grpc/policy-duration
branch
from
f074a70
to
0bf09a4
Compare
BenWhitehead
added
owlbot:ignore
gcf-owl-bot
bot
removed
the
owlbot:run
| 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.