search

Afischbacher / Nhl.Api

The Official Unofficial .NET NHL API
MIT License
25 stars 3 forks source link

Option to disable live updates for live feed #23

Closed andersme closed 2 years ago

andersme commented 2 years ago

I like that there is the live updating on the LiveGameFeedResult class, but this should at least be configurable to turn off, or slow down. I have certain use cases where I'm pulling in the live feed to download shot data and I don't need live updating. I do have another case where the live updating is nice, but 250ms is way too fast. If this could be fixed ASAP, that would be excellent.

andersme commented 2 years ago

It also appears that the task inside that object is not disposed and goes on forever.

Afischbacher commented 2 years ago

Hey @andersme,

Yes! This is something I definitely wanted to implement in future versions, so it's something I can put in 1.7.0.

As for the Task.Run inside the constructor of the LiveGameFeed that runs RaiseOnLiveGameFeedChangeEvent (which is what I think you are refferring too) the while loop does exit based on to many conditions such as game state and number of attempts. And for disposal of Task.Run, see here https://devblogs.microsoft.com/pfxteam/do-i-need-to-dispose-of-tasks/

My goal is to get this release out as soon as possible, sorry for the wait!

Thanks @andersme for all your hard work and suggestions they have been great!

andersme commented 2 years ago

We've just noticed that when we get a feed for a given game in our Blazor app, that task continues on forever. If we navigate back and forth to the page, more and more of those tasks are running until the app runs out of memory or kills our internet. There needs to be some way to cancel the task when disposing the feed object or something to that effect. I think this live updating being opt-in only would be a good start.

Afischbacher commented 2 years ago

Ah I see your point from that perspective, I see the issue, every time you navigate you to the page in your Blazor web application, it requests a new instance of the Live Game Feed Result, the problem is the number of attempts is quite long and is causing tasks to run for a long period of time, and as you refresh the page, the tasks keep getting spun up and persisted for a long period of time. Causing memory to be consumed quickly, I understand now from that perspective. Thanks for bringing it up. Best solution is implementing a CancellationToken for the event

Afischbacher commented 2 years ago

https://github.com/Afischbacher/Nhl.Api/pull/24

Afischbacher commented 2 years ago

Released v1.70! So I will close this issue :) 👍