-
Notifications
You must be signed in to change notification settings - Fork 34
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
aa storm feature / alert logic #1420
base: feature/add-new-AA-storm
Are you sure you want to change the base?
Conversation
Build succeeded and deployed at https://prism-1420.surge.sh |
alerting/cron_aa_storm_alert_run.sh
Outdated
## To set up the cron job, run the following command on the server: | ||
# crontab -e | ||
## and then add the following line to the crontab file: | ||
# 15 0,6,12,18 * * * ~/prism-app/alerting/cron_aa_storm_alert_run.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explain why you chose these times.
Tbh I think it's easier to just run hourly since we will have a check to see if there is "new" data to process or not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree, I will fix that and probably find a way to store persistently which was the latest leveraged reports.
@@ -0,0 +1,25 @@ | |||
import { isNaN } from 'lodash'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a docstring to this file please
@Max-Z80 please describe a bit more what this PR does in the PR body and adapt the name to what it is actually doing. There is no real "logic" here, it is simply initializing the new alert worker. No? |
THere is a logic in the aa-strom-alert-worker.js. I will describe deeper this logic in the PR body |
3b4c109
to
ddd4a08
Compare
export class CreateLatestAaStormReportsTable1738249210356 | ||
implements MigrationInterface | ||
{ | ||
public async up(queryRunner: QueryRunner): Promise<void> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do something a bit more future-proof here no?
I would add at least:
- emails (a list of emails)
- event_name (storm in our case)
- report_date
- activation_status
- last_triggered_at
- last_ran_at
implements MigrationInterface | ||
{ | ||
public async up(queryRunner: QueryRunner): Promise<void> { | ||
await queryRunner.query(`CREATE TABLE "latest_aa_storm_reports" ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's rename the table to anticipatory_action_alerts
Description
This is the logic for deciding whether to send an email or not.
the cron run hourly
latest reports for each storm are persisted in DB
when a new report is published and has not been processed yet by the alert system then an email payload might be built depending of the report status for example
The email payloads will then be used to create the emails and send it at the end of the process.
Notes
How to test the feature:
Checklist - did you ...
Test your changes with
REACT_APP_COUNTRY=rbd yarn start
REACT_APP_COUNTRY=cambodia yarn start
REACT_APP_COUNTRY=mozambique yarn start
Screenshot/video of feature: