Add slack notification #468
Conversation
| class SlackClient: | ||
| def __init__(self) -> None: | ||
| self.is_enabled = True | ||
| # Do we want a config in settings ? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #468 +/- ##
==========================================
- Coverage 84.79% 82.28% -2.52%
==========================================
Files 38 39 +1
Lines 1151 1236 +85
==========================================
+ Hits 976 1017 +41
- Misses 175 219 +44
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Hey there :)
So my suggestion:
And I have a question to understand the deeper need: is this for us to see when there is an alert or used by firefighters? Because we don't necessary need the GIF if that can be seen in the platform, perhaps only a link to the platform alert for gif visualization would be better (that way, we only share the static image URL + platform URL for action and GIF visualization) |
|
Hey, thanks for the feedback! I’m not sure I fully understand the issue here. Sure, OpenCV isn’t the lightest dependency, but it’s not exactly heavy either. Unless I’m missing something, we’re not particularly resource-constrained, are we? Do you have a concrete case in mind where this could be a problem? More broadly, I get the idea of keeping the API minimal, but I think we might be underestimating how valuable it is to quickly visualize an alert. Seeing a fire without a bounding box takes time. And since we’re doing early detection, the fire is often small and not obvious. A still image with a bbox is better, but a GIF is even quicker to interpret. We’re racing against time here If including OpenCV in the main container is really an issue, could we consider a second container dedicated to the computer vision part, just generating the GIF and returning a URL to the API? That would keep things modular while still providing the best user experience. Let me know what you think! |
|
Hey there :) Yup fully get your point, here is some short feedback:
For now my suggestion:
What do you think? |
|
Just to clarify: this is actually meant for firefighters on our Slack, the idea is for them to get alerts in real time on their phone or computer You're the API king haha, so I’ll let you make the call. I trust you, if you think it’s not the right direction long-term, then let's not go for it. Still, that GIF was pretty sexy! I guess we’ll need to kick off that vision API sooner rather than later. I really think there are quite a few interesting use cases we could develop around it @RonanMorgan sorry about that… I should’ve kicked off the conversation earlier. I made you work for nothing. Would you mind updating the PR to go back to a message with alert info and a URL instead? |
I thought they already had Telegram alerts? 🤔 Or do you mean non static ones? |
Give the possibility to activate slack notification into a channel (one channel per organisation)
=> Add an endpoint in the organizations/ path in order to activate the slack notification by registering the SLACK_HOOK
=> Create a SLACK service, similar to the Telegram service
=> modify the create_detection endpoint in on order to add the notification creation : the bbox is added to the images of the Sequence, a gif is created from the images of the Sequence, the gif is saved into the S3 bucket and the url is sent to slack.