Skip to content

fix: adding event query if start_at is null or not started#5

Merged
iefiru merged 4 commits into
mainfrom
fix/event-still-sent-after-started
Jul 16, 2026
Merged

fix: adding event query if start_at is null or not started#5
iefiru merged 4 commits into
mainfrom
fix/event-still-sent-after-started

Conversation

@rockleona

Copy link
Copy Markdown
Collaborator

Here is what I fixed from #4, I found the event query is lack of time statement, please review this change.

@rockleona rockleona requested review from chestercheng and iefiru May 19, 2026 12:54
@iefiru

iefiru commented May 24, 2026

Copy link
Copy Markdown
Collaborator

After testing, already-started events still show up in the report. It seems now_rows in _send_report_for_channel is not filtering by the slugs passed in via event_map.

@rockleona rockleona left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thank you @iefiru, I already added the filter to the query in this commit: 0f92ef3.

Comment thread src/argus/kktix/report.py Outdated

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Here the UPDATE sets last_reported_at for all events in the channel, including already-started ones. How about filtering by the slugs already passed in via event_map, and reusing the now timestamp from send_report() instead of calling utcnow_iso() again?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thank you @iefiru, I've pushed the commit per your input.

…ring

Co-authored-by: iefiru <7875793+iefiru@users.noreply.github.com>
@rockleona rockleona requested a review from iefiru July 13, 2026 08:46

@iefiru iefiru left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, Thanks!

@iefiru iefiru merged commit 0f80b40 into main Jul 16, 2026
1 check passed
@iefiru iefiru deleted the fix/event-still-sent-after-started branch July 16, 2026 02:11
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.

2 participants