Fix: Filtering fire alerts to reduce platform loading time

pyronear / pyro-platform

Detection & monitoring platform of wildfires

https://platform.pyronear.org/

Apache License 2.0

10 stars 11 forks source link

Fix: Filtering fire alerts to reduce platform loading time #73

Closed Akilditu closed 2 years ago

Akilditu commented 2 years ago

This PR aims at :

Trying to lighten ressources and loading time displaying only the last 5 alerts
Align dashboard devices last_ping timezone to FR

Enhancements & thoughts :

Still long to load while local testing this filters might be more relevant on API side @frgfm ?
Parsing code / doing unit testing on API fetches could be an option to determine more precisely what's actual costing time ? @pechouc ?

Thanks feedback and review

frgfm commented 2 years ago

Hey there 👋

So in the long-term, we will need to reduce the workload the platform imposes on the API. I will add a result limit (can only GET the last N events) on most API routes.

REgarding the platform, the CI isn't testing the modifications you proposed here so here is a suggestion:

we make independent functions to perform those operations
we add unittests for those
we use the functions in the actual code

That could be done in a PR later but we should do it at some point 😅

frgfm commented 2 years ago

So FYI everyone, we have already merged on the backend API side, the modification to only be able to fetch the last 50 entries instead of loading the whole table. That should speed things considerably

For the rest, including the "for" loops, I'll have to take a closer loop if we can optimize that a bit. But we for sure need to move forward with this PR

Akilditu commented 2 years ago

So FYI everyone, we have already merged on the backend API side, the modification to only be able to fetch the last 50 entries instead of loading the whole table. That should speed things considerably

For the rest, including the "for" loops, I'll have to take a closer loop if we can optimize that a bit. But we for sure need to move forward with this PR

Hi @frgfm thanks a lot for this, could you confirm the impacted route ?

frgfm commented 2 years ago

Hi @frgfm thanks a lot for this, could you confirm the impacted route ?

All the fetch routes (not update, delete, or get) benefit from this :+1: considering the size of tables, fetching media, alerts or events should be much faster now