pyronear / pyro-api

Alert Management API for wildfire prevention, detection & monitoring. Built with FastAPI & PostgreSQL
Apache License 2.0
21 stars 9 forks source link

feat: alert notification via websocket #235

Open blenzi opened 2 years ago

blenzi commented 2 years ago

This PR adds support for notifying clients such as pyro-platform instances of new alerts via websocket. Previously the platform had to periodically ping the API to ask if new alerts were sent.

codecov[bot] commented 2 years ago

Codecov Report

Merging #235 (896f1f1) into main (eb4fcc1) will increase coverage by 0.18%. The diff coverage is 82.35%.

@@            Coverage Diff             @@
##             main     #235      +/-   ##
==========================================
+ Coverage   95.04%   95.22%   +0.18%     
==========================================
  Files          60       61       +1     
  Lines        1393     1425      +32     
==========================================
+ Hits         1324     1357      +33     
+ Misses         69       68       -1     
Flag Coverage Δ
client 100.00% <ø> (ø)
unittests 94.97% <82.35%> (+0.19%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/app/api/deps.py 82.35% <25.00%> (-10.83%) :arrow_down:
src/app/api/connection_manager.py 100.00% <100.00%> (ø)
src/app/api/endpoints/alerts.py 97.10% <100.00%> (+0.49%) :arrow_up:

... and 3 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

blenzi commented 2 years ago

Websockets have many differences and subtleties with respect to HTTP routes. Without being able to test the setup locally, due to #217, #231, the development is quite hard.

The feature is implemented in a simple setup consisting of an API passing messages received via HTTP to websocket clients and a dash application (code) that receives those messages via websocket. Demo below.

The alternatives I see to move forward:

  1. Accept the PR basically as it is with the websocket route unprotected, meaning any one can listen to the alerts (but not posting, like before). This allows implementing the feature in pyro-platform as soon as it is deployed
  2. Wait for #217, #231 to be able to test the setup locally
  3. Port the security implementation to the test-API above, which can be long and maybe never exactly like this API so back to 1. or 2.

Any suggestions? @frgfm ?

websocket

frgfm commented 1 year ago

Before diving into a review, and iterating on this, I want to address something here: