Support the time crate in addition (instead of?) chrono

[tasn]

It looks like chrono has security issues that can't be fixed. It depends on an old version of the time library which is good on its own anyway. Should we add support for the time crate?

Is there a way for me to do it in our own code before support hits sea-orm?

More info: svix/rust-ksuid#1

[billy1624]

Hey @tasn, thanks for the suggestion! I think it's more future proof to adopt time.

Related PRs

• with-time sea-query#254
• Add "with-time" feature sea-query#256
• time support #508

Shout-out to @cemoktra!

[tasn]

Ha, I thought I missed those PRs, but I opened this ticket before these PRs were opened. :P

[cemoktra]

You can take my starts and finalize it, I'm busy with different stuff right now but also interested in this.

[billy1624]

You can take my starts and finalize it, I'm busy with different stuff right now but also interested in this.

I will, if I have the time to do so.

Others are welcome to contribute :)

[tyt2y3]

The alternative might be, is it possible to fix chrono?
I think a potential confusion is that time and chrono will be mutually exclusive.
And so if a downstream stream crate chooses to use time and another one chooses to use chrono, the project will not be able to compile.

[tasn]

According to the link and information I was provided with (see the link above): no, chrono can't be fixed. Also, they wouldn't be mutually exclusive, only in the automatic code generation but even then it can be a flag.

So I think runtime is find, the only incompatibility would be seaorm-cli's generation which is not a compatibility problem.

[tyt2y3]

Yeah I agree. We should go down the route of supporting both "together" (without being mutually exclusive).
We can expose generic alias to the types DateTime and specific types ChronoDateTime something like that

[tasn]

That's great for longer term, but even just having things work when I manually set a time object as a seaorm field would go a very long way. We can't do it (without a wrapper type) because the trait and time are both external to trait. So just built in conversion definitions in searorm is all we need.

[billy1624]

Yeah I agree. We should go down the route of supporting both "together" (without being mutually exclusive). We can expose generic alias to the types DateTime and specific types ChronoDateTime something like that

So, we'll keep all existing type aliases untouched.

sea-orm/src/entity/prelude.rs

Lines 18 to 37 in 0a26dee

	#[cfg(feature = "with-chrono")]
	pub use chrono::NaiveDate as Date;
	#[cfg(feature = "with-chrono")]
	pub use chrono::NaiveTime as Time;
	#[cfg(feature = "with-chrono")]
	pub use chrono::NaiveDateTime as DateTime;
	/// Date time with fixed offset
	#[cfg(feature = "with-chrono")]
	pub type DateTimeWithTimeZone = chrono::DateTime<chrono::FixedOffset>;
	/// Date time represented in UTC
	#[cfg(feature = "with-chrono")]
	pub type DateTimeUtc = chrono::DateTime<chrono::Utc>;
	/// Date time represented in local time
	#[cfg(feature = "with-chrono")]
	pub type DateTimeLocal = chrono::DateTime<chrono::Local>;

Then, we'll add chrono & time version of all these type aliases. Such that we've... for DateTime.

// Existing alias
#[cfg(feature = "with-chrono")] 
pub use chrono::NaiveDateTime as DateTime; 

// New alias
#[cfg(feature = "with-chrono")] 
pub use chrono::NaiveDateTime as ChronoDateTime;

// New alias
#[cfg(feature = "with-time")]
pub use time::PrimitiveDateTime as TimeDateTime;

[tasn]

That's fine, I don't think there's a problem with this. Let people choose what they want to use.

The main thing though is that I can't even reimplement it in our own project because I can't add the necessary traits on PrimitiveDateTime, so we are just at your mercy, or alternatively need to define our own wrapper types.

[billy1624]

Hey @tasn, I'll open a PR and implement the supports for time crate in SeaORM & SeaQuery :P

[tasn]

Thanks a lot @billy1624! 🙏🏻

[tasn]

Yay, thanks a lot!

[cemoktra]

Hey, while its nice that this got into the new release it would be helpful to add a flag to the code generator to specify if it should generate for chrono or time

[billy1624]

Good suggestions! @cemoktra Would you mind opening a feature request for this?