Skip to content

Fix parsing of timestamps with newer lava#33

Merged
sjoerdsimons merged 1 commit into
mainfrom
fix-date-parsing
Jun 23, 2026
Merged

Fix parsing of timestamps with newer lava#33
sjoerdsimons merged 1 commit into
mainfrom
fix-date-parsing

Conversation

@sjoerdsimons

Copy link
Copy Markdown
Contributor

In the latest version of lava the dt field is created with atetime.datetime.now(datetime.UTC).isoformat() rather then datetime.datetime.utcnow().isoformat(). This causes the timezone specifier to be added (always +00:00 as the logs are UTC)'.

From a Rust perspective the former is a NaiveDateTime (no timezone) while the later is a DateTime<Utc>. Adjust the parsing to try both, but convert to NaiveDateTime to keep the same user API. In future potentially this could be moved to always be a DateTime to make it more clear to the end-user what the stamp is

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts JobLogEntry.dt deserialization to handle newer LAVA log timestamps that include an explicit UTC offset (e.g. +00:00), while preserving the existing NaiveDateTime API exposed by this crate.

Changes:

  • Added a custom Serde deserializer that accepts both NaiveDateTime and DateTime<Utc> and normalizes to NaiveDateTime.
  • Annotated JobLogEntry.dt to use the new deserializer.
  • Added a unit test asserting that timestamps with and without +00:00 deserialize to the same NaiveDateTime.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lava-api/src/joblog.rs Outdated
In the latest version of lava the dt field is created with
atetime.datetime.now(datetime.UTC).isoformat() rather then
datetime.datetime.utcnow().isoformat(). This causes the timezone
specifier to be added (always +00:00 as the logs are UTC)'.

From a Rust perspective the former is a `NaiveDateTime` (no timezone)
while the later is a `DateTime<Utc>`. Adjust the parsing to try both,
but convert to NaiveDateTime to keep the same user API. In future
potentially this could be moved to always be a DateTime to make it more
clear to the end-user what the stamp is
@sjoerdsimons sjoerdsimons added this pull request to the merge queue Jun 23, 2026
Merged via the queue into main with commit c5ea94d Jun 23, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants