Stop channel bug fix and reload frequency #20425
Conversation
sharon-fdm
added
~unreleased bug
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
- Fixed bug in
ee/server/calendar/google_calendar.goto ensure event channel stops only once - Improved reliability of Google Calendar integration by proper cleanup of event watches
- Adjusted reload frequency for calendar events in
server/cron/calendar_cron.gofrom 12 hours to 30 minutes - Reordered import statements in
server/cron/calendar_cron.go
2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
| @@ -350,6 +348,14 @@ func (c *GoogleCalendar) GetAndUpdateEvent(event *fleet.CalendarEvent, genBodyFn | |||
| } | |||
| } | |||
|
|
|||
| // If event was deleted/cancelled, we need to stop watching it | |||
| if !channelStopped { | |||
There was a problem hiding this comment.
@getvictor Are you sure there are no paths where the function returns and this will not be called even if it's needed?
Maybe consider putting under defer?
There was a problem hiding this comment.
I think it is OK as is -- I reviewed the code. I will rework the code so that we don't stop/create a new channel to watch the calendar if event changes. Maybe that will help with callback latency. That change can go into the next release/patch.
There was a problem hiding this comment.
PR Summary
(updates since last review)
- Moved removal of calendar event from queue to post-database update in
/server/cron/calendar_cron.go - Adjusted reload frequency for calendar events to 30 minutes in
/server/cron/calendar_cron.go
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
| // If event was deleted/cancelled, we need to stop watching it | ||
| if !channelStopped { | ||
| err = c.config.API.Stop(details.ChannelID, details.ResourceID) | ||
| if err != nil { | ||
| level.Warn(c.config.Logger).Log("msg", "stopping Google calendar event watch", "err", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Could you explain the change? It seems there are two paths where the watching of the event is not stopped (all day events and one that says // Delete this event to prevent confusion. This operation should be rare. ). Were any of these two scenarios causing issues?
There was a problem hiding this comment.
The bug was when we deleted the event from Google calendar, we did not stop watching the notification channel. When event is deleted, it comes back from Google calendar with status = "cancelled". With this change, we will now be stopping the notification channel for that case.
Unreleased bug fix for #19352