Add crate for integration with the AWS SDK for Rust

[snaggen]

I have implemented some basic conversion from the AWS Smithy type DateTime into/from Timestamp and Zoned.

[robertbastian]

Wouldn't this be better in aws_smithy_types_convert like the chrono and time conversions?

[snaggen]

I created a PR for that previously, but they didn't want to add it since jiff isn't stable yet.

[BurntSushi]

Ref smithy-lang/smithy-rs#3826

I'll get to reviewing this soon, after I get through the batch of changes related to core-only work.

[snaggen]

Ref smithy-lang/smithy-rs#3826

I'll get to reviewing this soon, after I get through the batch of changes related to core-only work.

No, worries.... it is just some simple convenience helpers, so it is not like my code is dependent on this :)

[fiadliel]

The nanosecond portion of a jiff Timestamp are negative for times before the Unix epoch (it ranges between +1s and -1s), this should be taken into account (e.g. the type casts between u32 and i32 are incorrect).

[snaggen]

The nanosecond portion of a jiff Timestamp are negative for times before the Unix epoch (it ranges between +1s and -1s), this should be taken into account (e.g. the type casts between u32 and i32 are incorrect).

The smithy datetime is defined as seconds and nano seconds since unix epoch, so a simple check for that should fix this. I will have a look at it later.

[snaggen]

@fiadliel Ok, it seems that I was wrong about DateTime being only positive. But I have added nano second conversions for times before epoc. Please have a look and see if it looks correct to you. I checked for conversion the other way also, but that seems to be handled by jiff automatically.

[snaggen]

Added some tests and ensured the calculations are correct for negative epoch

[fiadliel]

I think that fixes the calculation around times before 1970. Thanks!

One last thing: I think you should reconsider being able to convert DateTime into Zoned (or remove Zoned altogether), ref. the opening comment in this PR: #240 where Zoned support was left out of the jiff-sqlx crate.
But look, I'm just a drive-by commenter, so it's not an official request :)

[snaggen]

I don't really have any strong opinions about including Zoned or not. But I added it since this is to make things a bit more convenient, not having to jump through a Timestamp first. And it is clearly documented that it creates a Zoned for timezone UTC, so I don't really see any issues with having it included.

[snaggen]

@BurntSushi have you had time to look at this?

[BurntSushi]

Thanks for the ping.

In terms of whether I'm willing to bring this crate on and maintain it indefinitely, I'm somewhat unsure. It does look pretty lightweight. On the other hand, the smithy-lang people seem on board with supporting Jiff once it's 1.0, and that shouldn't be too far away.

The other consideration here is the complexity of the actual conversion. For icu, for example, the conversions are not just instants in time, but civil times as well, and those have more failure points. But here, the only relevant target type is aws_smithy_types::date_time::DateTime, which is an instant in time. Which means I don't think an entire crate for this is really buying us much.

How would you use a crate like this? Is this something where you could define a helper function on your own and just use that? I also don't have a good sense of how many people would actually benefit from this, which makes it hard to determine if this is worth maintaining indefinitely on my end.

Separately from the question of whether we should do this, the actual change here would need some work. Specifically:

• It should follow the API design pattern established in jiff-icu.
• We shouldn't have conversions to Zoned. A conversion that automatically uses UTC isn't buying much, and that sort of thing really should be determined by the caller. I would in theory be okay with adding this conversion in the future if there was strong evidence that it was very common and folks were stumbling on it though.

[snaggen]

Fair points! I'll wait for 1.0 and try to add it to the smithy-lang crate

And about usage, sure this could easily be a helper function you define by your self. However, it is always convenient to get something that is tested, especially when there are non-obvious cases like before epoch where you have to recalculate things a little bit.

[BurntSushi]

Another point of clarity here is that I'm definitely not trying to be a receptacle of Jiff integrations with other crates. I did SQLx and Diesel because I perceived folks were bouncing off of the lack of Jiff integration for those specific crates and because they were unwilling to add upstream support, and I did icu because I plan for icu to be a paved path toward non-Gregorian calendar support and localized formatting. The story for SQLx and Diesel integration is a fair bit more compelling, because if you don't have a Jiff integration, you end up needing to write a fair amount of arcane boiler plate to get it to plug into the SQLx/Diesel machinery. It's very much non-trivial.

Fair points! I'll wait for 1.0 and try to add it to the smithy-lang crate

All righty, thanks. I'll close this out then. If this doesn't end up working, please feel free to swing back around here and we can re-evaluate.

And I do acknowledge that having a tested conversion routine is beneficial. For me, it's more about whether there's enough people hitting this that it's worth me maintaining in order to overcome what I hope is a short-term gap. If, once Jiff is 1.0 and the smithy folks still won't add support for it (and it seems like they would, or at least, that's I think what they telegraphed), then I'd be more open to supporting this longer term.