Use django-tasks for sending notifications

[hmpf]

Scope and purpose

Might eventually close #166, #359, #400.

Use django-tasks to send notifications instead of a background process. Currently uses django signals to enqueue tasks on Event create. With the database backend we could use a trigger to add a task on database save of an event!

Has one task to check whether to send notifications and another to actually send the notification.

Test by for instance having a filter that triggers on argus as source 24/7 and sends an email. Turn notifications on (SEND_NOTIFICATIONS=True in settings or use the environment variable SEND_NOTIFICATIONS=yes) and use a dummy email backend (EMAIL_BACKEND = "django.core.mail.backends.console.EmailBackend" in settings).

This pull request

• adds/changes/removes a dependency

Contributor Checklist

Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to Argus can be found in the
Development docs.

• Added a changelog fragment for towncrier
• Added/amended tests for new/changed code
• Added/changed documentation, including updates to the user manual if feature flow or UI is considerably changed
• Linted/formatted the code with ruff and djLint, easiest by using pre-commit
• The first line of the commit message continues the sentence "If applied, this commit will ...", starts with a capital letter, does not end with punctuation and is 50 characters or less long. See our how-to
• If applicable: Created new issues if this PR does not fix the issue completely/there is further work to be done
• [ ] If this results in changes in the UI: Added screenshots of the before and after
• If this results in changes to the database model: Updated the ER diagram

[github-actions]

Test results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit beef6b8.

♻️ This comment has been updated with latest results.

[lunkwill42]

So, in short, this appears to refactor the existing in-process notification handler by reframing it as a django-tasks task with the ImmediateBackend.

Lots of questions:

• I assume the end goal is to replace the ImmediateBackend with DatabaseBackend and a db_worker management command? (Or perhaps our own custom backend?)

• Why is it a good idea to split 'check for notifications' and 'send notifications' into separate tasks?

• Have you figured out how to transition from django-tasks to Django 6?

• You mentioned something in IRL that I interpreted as "the db_worker uses database polling". This is why I became averse to the django-q2-based PoC. Could we add NOTIFY/LISTEN support to the db_worker or would we have to write our own? I do realize that a NOTIFY/LISTEN-based mechanism would be PostgreSQL specific - but then again, at what point are we not tied to PostgreSQL already?

Nevertheless, the refactor so far looks quite neat and clean, and I'm glad to see we can eventually accomplish this without the need for a bunch of 3rd party libraries or components :)

[hmpf]

So, in short, this appears to refactor the existing in-process notification handler by reframing it as a django-tasks task with the ImmediateBackend.

Lots of questions:

* I assume the end goal is to replace the `ImmediateBackend` with `DatabaseBackend` and a `db_worker` management command? (Or perhaps our own custom backend?)

Yes. We ought to document using DatabaseBackend.

* Why is it a good idea to split 'check for notifications' and 'send notifications' into separate tasks?

Because

1. we will be adding more to do on the check-step (checking for planned maintenance), making it increasingly expensive, and I want to save the incident ASAP
2. actually sending the notification in a separate step makes it "easy" to delay sending notifications, for instance for SMSes. We could have a queue with notifications that needs to be bunched up. We have issues for that already..
3. it looks sooo clean...

* Have you figured out how to transition from `django-tasks` to Django 6?

Eight-ball says: too soon to say.

RealOrangeOne/django-tasks#204

* You mentioned something in IRL that I interpreted as "the db_worker uses database polling".  This is why I became averse to the django-q2-based PoC.  Could we add NOTIFY/LISTEN support to the db_worker or would we have to write our own?  I do realize that a NOTIFY/LISTEN-based mechanism would be PostgreSQL specific - but then again, at what point are we not tied to PostgreSQL already?

I thought your main beef with django-q2 was that it used pickle? django-tasks slings around json-blobs, to the despair of people who very much prefer pickle.

https://github.com/RealOrangeOne/django-tasks/discussions/85

I don't see a problem with poll, it works very well if argus sees as much traffic as I fear it might in the future. How well would NOTIFY/SUBSCRIBE handle the mist spam, lots of incidents at the same second?

Nevertheless, the refactor so far looks quite neat and clean, and I'm glad to see we can eventually accomplish this without the need for a bunch of 3rd party libraries or components :)

[hmpf]

https://www.enterprisedb.com/blog/listening-postgres-how-listen-and-notify-syntax-promote-high-availability-application-layer

There’s still a kind of flaw with relying on NOTIFY events from Postgres: they’re ephemeral. If we stopped the listener app for any reason, nobody would receive notifications at all. If we later started the app again, notifications would resume, but any missed notifications would be lost forever. There’s also a small stampede problem if we want multiple iterations of the notification app to run simultaneously. We understand that post notifications are hardly critical data, but we can do better.

Goes on to describe polling with "FOR UPDATE SKIP LOCKED", which is exactly what django-tasks' database backend does...

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[CLAassistant]

All committers have signed the CLA.

[hmpf]

Switching to django 6.0 is seddable and would entail:

• changing the app from django_task to django.task in INSTALLED_APPS
• import from django.task instead of django_task in our code

It wouldn't change anything in the database, that won't happen until/if the database backend is included with django.

What would need at least to happen then is:

• update the app "django_tasks.backends.database" in INSTALLED_APPS to whatever new name it gets, I hope the schema will not be part of the django.tasks app but be optional
• if the app label changes there will be a migration of some sort

[aleksfl]

Seems to run as it should, although I am still testing how to send the notifications properly. Some minor comments but nothing important. Code looks good in general.

[aleksfl]

I'm assuming this is a necessary change? Might be worth noting on the PR for clarity.

[lunkwill42]

Ok, so I gave this a once-over with manual testing with the database backend, and it appears to work ok, but I did find three issues: Two related to this PR and also a more troubling one, that appears also on main.

1. It seems that this PR does not adhere to the SEND_NOTIFICATIONS setting. It sends notifications regardless. Did I miss something?
2. Completed tasks are left in the queue and litter the database. I can no longer find the django-tasks documentation online, apparently because it subsumed into Django. The README mentions pruning old tasks from the database: https://github.com/RealOrangeOne/django-tasks?tab=readme-ov-file#pruning-old-tasks . This is not mentioned in the Django 6 docs, as far as I can see, but the problem is still there. We should perhaps document how users should set up a cron job to prune old tasks from the queue.
3. I could not get Argus to send notifications when I made other incident changes (i.e. acknowledging, closing etc.), so I switched back to main and observed the same behavior there. This is troubling. Any idea what has changed, when? Am I doing something wrong?

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud