Skip to content

Conversation

johannaengland
Copy link
Contributor

@johannaengland johannaengland commented Mar 17, 2023

After merging #2413 and everything related to it (#2515, #2545, #2560, #2565, #2580) it happened occasionally that notifications were sent out with the error

no templates defined for <AlertGenerator: source=<Subsystem: ipdevpoll> device=<Device: redacted> netbox=None subid='' time=datetime.datetime(2023, 3, 17, 9, 41, 24, 79802) event_type=<EventType: deviceNotice> state='x' value=100 severity=3 history_vars={} alert_type=None varmap={}>, sending generic alert message

We could pinpoint that this happened due to the eventengine being notified about a new event in the EventQueue and processing it without the varmap being saved yet, which lead to this error, since the alert type is saved in the varmap.

Making the save function atomic prevents this from happening since the eventengine only starts processing the event when all information is saved properly.

These save methods will save up to multiple objects to two separate models in one operation and we need to ensure that these related models are stored in the same transaction

Without it this transaction the event notification sent by postgreSQL will be send before the related varmap model is saved
@johannaengland johannaengland self-assigned this Mar 17, 2023
@johannaengland johannaengland changed the base branch from master to 5.6.x March 17, 2023 09:41
@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

Merging #2594 (0dbf350) into 5.6.x (238fb8f) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            5.6.x    #2594      +/-   ##
==========================================
+ Coverage   53.72%   53.76%   +0.03%     
==========================================
  Files         558      558              
  Lines       40587    40590       +3     
==========================================
+ Hits        21806    21822      +16     
+ Misses      18781    18768      -13     
Impacted Files Coverage Δ
python/nav/models/event.py 80.68% <100.00%> (+0.15%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions
Copy link

github-actions bot commented Mar 17, 2023

Test results

     12 files       12 suites   12m 8s ⏱️
3 220 tests 3 124 ✔️   96 💤 0
9 135 runs  8 847 ✔️ 288 💤 0

Results for commit 0dbf350.

♻️ This comment has been updated with latest results.

@lunkwill42 lunkwill42 added the bug label Mar 17, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link
Member

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 💯

@lunkwill42 lunkwill42 merged commit 269f506 into Uninett:5.6.x Mar 22, 2023
@johannaengland johannaengland deleted the bugfix/atomic-save-of-alerts-and-events branch March 27, 2023 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants