grpc: Read timestamps from timestamppb fields instead of int64 fields #7121
Conversation
|
|
There was a problem hiding this comment.
Looks great!
This makes me think we need some sort of guidance on how to express "optional" timestamps. Something like this:
Timestamps in protocol buffers must always be expressed as timestamppb.Timestamp. Timestamps must never be zero, in the sense of timestamp.AsTime().AsZero(). When a timestamp field is optional, absence must be expressed through the absence of the field, rather than presence with a zero value.
Senders must check that timestamps are non-zero before sending them. Receivers must check that timestamps are non-zero before accepting them.
@jsha Where do you see this text being added? I think either |
|
CONTRIBUTING.md IMO |
|
I'll get that added in the next PR which is a big cleanup of all the old int64 timestamp fields. |
pgporada
deleted the
i-can-say-that-i-dont-know-what-im-doing-but-i-cant-say-i-have-the-time
branch
This is a cleanup PR finishing the migration from int64 timestamps to protobuf `*timestamppb.Timestamps` by removing all usage of the old int64 fields. In the previous PR #7121 all fields were switched to read from the protobuf timestamppb fields. Adds a new case to `core.IsAnyNilOrZero` to check various properties of a `*timestamppb.Timestamp` reducing the visual complexity for receivers. Fixes #7060
Switch to reading grpc timestamp values from the new timestamppb protofbuf fields, completely ignoring the old int64 fields.
Part 3 of 4 for #7060