Use core::time::Duration instead of time::Duration

[myOmikron]

Hi,

is there a particular reason that continuously_delete_expired takes core's Duration and Expiry::OnInactivity taking time::Duration?

My example code:

    let store = DBSessionStore {};
    tokio::spawn(async move {
        store
            .continuously_delete_expired(Duration::from_secs(5 * 60))
            .await
    });

    let app = Router::new()
        .layer(
            ServiceBuilder::new()
                .layer(HandleErrorLayer::new(|_: BoxError| async {
                    StatusCode::INTERNAL_SERVER_ERROR
                }))
                .layer(
                    SessionManagerLayer::new(store)
                        .with_expiry(Expiry::OnInactivity(time::Duration::new(-1440 * 60, 0))),
                ),
        );

In my opinion it would make more sense to take core::time::Duration in the public API, because:

• You are not forcing the user to use time
• time::Duration accepts negative durations which wouldn't make sense in an expiry setting

I haven't looked through the complete public API, but I'd suggest unifiying the Durations where it make sense.

[maxcountryman]

This is because tokio has its own representation of time which interval uses. Because we use interval, we use tokio's Duration.

As far as I understand, core::time is not an area of focus for improvement and as such it's encouraged to use a third party crate, like time instead.

If we wanted to narrow the public API surface, I'd be open to converting from time to core::time within the deletion task itself (it's not immediately obvious to me if that's possible).

[myOmikron]

If we wanted to narrow the public API surface, I'd be open to converting from time to core::time within the deletion task itself (it's not immediately obvious to me if that's possible).

Not without loosing precision in a way or another. time provides unsigned_abs for doing this conversion.

[maxcountryman]

I think that's an okay trade off in this case.