-
-
Notifications
You must be signed in to change notification settings - Fork 80
feat(apalis-sql): add optional time crate support #655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(apalis-sql): add optional time crate support #655
Conversation
- Add chrono/time feature flags for datetime library selection - Replace direct chrono usage with SqlDateTime and SqlDateTimeExt - Remove hardcoded chrono dependency in favor of feature-gated support - Update to apalis-sql path dependency for datetime abstraction Related to apalis-dev/apalis#655
Migrate to SqlDateTime/SqlDateTimeExt from apalis-sql instead of direct chrono/time usage. Adds migration to convert DATETIME columns to TIMESTAMP for proper UTC handling. Related to: apalis-dev/apalis#655
geofmureithi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH This looks neat 🚀
Just the small change to move it to its own module
You could also add some module level docs to explain the logic behind it.
|
@Himmelschmidt |
|
I'll try to get this done over the next day or two |
Make chrono optional (still default) and add time crate as alternative datetime backend via feature flags.
Ensures --all-features works correctly in CI.
Aligns with sqlx behavior. Updates CI to require at least one datetime feature.
Adds SqlDateTimeExt extension trait that provides: - now() - returns current UTC datetime - to_unix_timestamp() - returns Unix timestamp This allows downstream crates to use `SqlDateTime::now()` without any conditional compilation, abstracting over chrono/time.
Adds from_unix_timestamp() method to convert Unix timestamps back to SqlDateTime, completing the bidirectional conversion API.
Address PR review feedback: - Rename sql_datetime module to datetime - Update module doc comment to "DateTime abstraction for unified time handling" - Remove unnecessary ignore from doc test - Fix clippy use_self warnings
ad70b66 to
0af7ad7
Compare
geofmureithi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
I ran |
* feat: use SqlDateTime abstraction from apalis-sql - Add chrono/time feature flags for datetime library selection - Replace direct chrono usage with SqlDateTime and SqlDateTimeExt - Remove hardcoded chrono dependency in favor of feature-gated support - Update to apalis-sql path dependency for datetime abstraction Related to apalis-dev/apalis#655 * chore: update apalis deps to git and add get_queue impl * bump: update apalis deps to rc.2 * chore: use crates.io deps instead of git * refactor: simplify get_queue impl * chore: update deps and changelog * revert: get_queue change * fix: use PgPool for PgContext type alias
Replace direct chrono dependency with DateTime/DateTimeExt from apalis-sql, enabling support for both chrono and time datetime libraries via feature flags. Changes: - Add `chrono` (default) and `time` feature flags - Use DateTimeExt::from_unix_timestamp instead of chrono::Utc.timestamp_opt - Update apalis dependencies to 0.7.0-rc.2 - Move chrono to dev-dependencies (only needed for tests/examples) Related: apalis-dev/apalis#655
Replace direct chrono dependency with DateTime/DateTimeExt from apalis-sql, enabling support for both chrono and time datetime libraries via feature flags. Changes: - Add `chrono` (default) and `time` feature flags - Use DateTimeExt::from_unix_timestamp instead of chrono::Utc.timestamp_opt - Update apalis dependencies to 0.7.0-rc.2 - Add migration to convert datetime columns to TIMESTAMP - Move chrono to dev-dependencies (only needed for tests/examples) Related: apalis-dev/apalis#655
Adds feature flags for
chrono(default) andtimecrate support in apalis-sql.See https://github.com/orgs/apalis-dev/discussions/649
Changes
SqlDateTimetype alias that resolves based on enabled featureSqlDateTimeExttrait withnow(),to_unix_timestamp(),from_unix_timestamp()Usage