Missing Builder derive for MillisecondTimestamp?

[DirkRusche-Otto]

I was trying to instantiate a KafkaRecord and the builder exposes a timestamp method that lets me pass a MillisecondTimestamp. But unfortunately I couldn't find a way to construct the MillisecondTimestamp struct (without using Deserialize - which feels a bit awkward to use).

So I guess there is a #[cfg_attr(feature = "builders", derive(Builder))] missing?

[jlizen]

Yeah, looks like they are needed here:
https://github.com/aws/aws-lambda-rust-runtime/blob/main/lambda-events/src/encodings/time.rs

Another option is to just chain through chrono setters for from_systemtime_and_offset or other ergonomic constructors. Technically the non exhaustive signals that we might add future fields, but that seems unlikely to me, and they would surely be optional.

PR welcome!

[DirkRusche-Otto]

Thanks for your quick response. I can prepare a PR, can you give me more precise instructions what solution you would prefer?

[jlizen]

We definitely want derive(Builder), like you suggested.

If you find the ergonomics for setting it poor, I don't mind MillisecondTimestamp::from_systemtime_and_offset() as a supplementary change. Up to you if you want to add it, I'd accept it.

If you don't mind adding the same for builder / optionally the direct constructor to the other types (SecondTimestamp, etc), I think they need it too.

[DirkRusche-Otto]

Here you go #1138

[github-actions]

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.