Skip to content

[LFXV2-882] Fix date parsing to log warnings instead of returning errors#19

Merged
emsearcy merged 2 commits intomainfrom
andrest50/fix-leave-time
Dec 9, 2025
Merged

[LFXV2-882] Fix date parsing to log warnings instead of returning errors#19
emsearcy merged 2 commits intomainfrom
andrest50/fix-leave-time

Conversation

@andrest50
Copy link
Contributor

@andrest50 andrest50 commented Dec 9, 2025

Summary

  • Changed date parsing error handling to log warnings instead of returning errors
  • Prevents valid participant data from being dropped due to malformed date strings
  • Added additional context to warning logs for better debugging
  • Bumped chart version to 0.3.6

Ticket

LFXV2-882

Changes

  • handlers_meetings.go:
    • Modified convertInviteeToV2Participant to log warnings for CreatedAt/ModifiedAt parsing errors
    • Modified convertAttendeeToV2Participant to log warnings for CreatedAt/ModifiedAt/JoinTime/LeaveTime parsing errors
    • Added invitee_id, attendee_id, and meeting_and_occurrence_id to warning logs
    • Only set date fields when parsing succeeds
  • Chart.yaml: Bumped version from 0.3.5 to 0.3.6

Behavior Change

Previously, when date fields couldn't be parsed as RFC3339, the entire participant record would fail to sync with an error. Now, the system logs a warning and continues processing, allowing the participant data to be indexed without the problematic date fields.

🤖 Generated with Claude Code

- Change CreatedAt, ModifiedAt, JoinTime, and LeaveTime parsing errors from returning errors to logging warnings
- Continue processing participants even when date fields cannot be parsed
- Add additional context (IDs) to warning logs for better debugging
- Bump chart version to 0.3.6

This prevents valid participant data from being dropped due to malformed date strings in the v1 system.

Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: Andres Tobon <andrest2455@gmail.com>
@andrest50 andrest50 marked this pull request as ready for review December 9, 2025 18:39
@andrest50 andrest50 requested review from a team and emsearcy as code owners December 9, 2025 18:39
Copilot AI review requested due to automatic review settings December 9, 2025 18:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a critical data synchronization issue where malformed date strings in Zoom meeting participant data would cause entire participant records to be dropped. By changing date parsing failures from errors to warnings, the system now logs the issue and continues processing, allowing participant data to be indexed without problematic date fields.

Key Changes:

  • Modified error handling in date parsing from returning errors to logging warnings
  • Added contextual identifiers (invitee_id, attendee_id, meeting_and_occurrence_id) to warning logs for better debugging
  • Bumped Helm chart version to reflect the bug fix

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
cmd/lfx-v1-sync-helper/handlers_meetings.go Updated convertInviteeToV2Participant and convertAttendeeToV2Participant to log warnings for date parsing failures instead of returning errors, allowing partial data to be processed
charts/lfx-v1-sync-helper/Chart.yaml Bumped chart version from 0.3.5 to 0.3.6 to reflect the bug fix

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

Copy link
Contributor

@mauriciozanettisalomao mauriciozanettisalomao left a comment

Choose a reason for hiding this comment

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

Just a minor comment: an idea to try loading v1 data into v2 as much as possible.

}

if invitee.ModifiedAt != "" {
modifiedAt, err := time.Parse(time.RFC3339, invitee.ModifiedAt)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not related to the change you made, but I'm assuming our source data is kinda inconsistent with the dates, so maybe we could try different date formats to actually get the data into v2 as much as possible.

Here's an example (not elegant at first glance, I know, but it works when the source doesn't follow a pattern):

Maybe this could be a backlog ticket.

Copy link
Contributor

Choose a reason for hiding this comment

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

that's a good idea, though in this case somehow that field actually got the value left the meeting. Reason : left the meeting (the leave reason being stored as the leave time, not sure if it's itx-zoom's fault or bad data in the Zoom webhook itself.

Also I believe Meltano is actually be doing some date normalization already. I just checked the autodetected/built Dynamo ingest scheme, and these are getting considered as datetime fields—and while we turned off schema validation for Dynamo-replicated data, because it was so inconsistent—meaning individual fields are coming across with different schemas, so some records will be a "string" instead of a "datetime"—the ones that do get parsed as a datetime are, I believe, converted to python datetime.datetime in transit, and then back to JSON.

emsearcy
emsearcy previously approved these changes Dec 9, 2025
Fix issue in SLSA generation. Also update to latest cosign (unrelated,
but I was holding off until I had a chance to understand what, if any,
breaking changes might affect us in Cosign v3).

Signed-off-by: Eric Searcy <eric@linuxfoundation.org>
@emsearcy emsearcy dismissed stale reviews from mauriciozanettisalomao and themself via 59a88c5 December 9, 2025 20:04
@emsearcy emsearcy self-requested a review December 9, 2025 20:04
@emsearcy emsearcy merged commit e4fc57e into main Dec 9, 2025
3 checks passed
@emsearcy emsearcy deleted the andrest50/fix-leave-time branch December 9, 2025 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants