Skip to content
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

Fix SessionStart double notifications #272

Merged
merged 5 commits into from
Sep 5, 2024

Conversation

chiragkrishna
Copy link
Contributor

Adds a 5 second time gap between events to prevent the notification being sent twice. fixes #269

Adds a 5 second time gap between events to prevent the notification being sent twice.
fixes jellyfin#269
Corrected Nested 'if' statements.
Copy link
Member

@crobibero crobibero left a comment

Choose a reason for hiding this comment

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

Need a way to remove old values from the cache or this dictionary could grow very large.

to prevent timer from initializing every time the SessionStartedEventArgs event is triggered
@@ -26,6 +32,7 @@ public SessionStartNotifier(
{
_applicationHost = applicationHost;
_webhookSender = webhookSender;
_cleanupTimer?.Change(0, (int)TimeSpan.FromMinutes(30).TotalMilliseconds);
Copy link
Member

Choose a reason for hiding this comment

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

Why is this .Change() needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

technically that line is redundant, just to avoid compiler warnings
Unused field '_cleanupTimer'CA1823

@chiragkrishna
Copy link
Contributor Author

do you have a better solution or idea for this?

@crobibero
Copy link
Member

I think it might be enough to clean up old cached sessions when a new SessionStart event is triggered so we don't need to have this timer thread running in the backround

run cleanup when the event is triggered
@crobibero crobibero merged commit f4b40c6 into jellyfin:master Sep 5, 2024
4 checks passed
@chiragkrishna chiragkrishna deleted the patch-1 branch September 7, 2024 18:51
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.

SMTP notification for "SessionStart" sends two emails
2 participants