Fix missing previous xhardstate - #857
Conversation
…rt name in order to reflect the undefined state
|
Thank you for your contribution. This pull request has been marked as stale as it has not passed the automated tests and there was no activity for the last 14 days. This pull request will be closed due to inactivity after 60 days, if no action is taken. |
|
for the record: even after bringing the PR branch up to date with master, the test fail due to some setup errors, not caused by the changes made in the PR |
|
Same issue here. |
|
Hi @zerwes,
Yes, I understand. I just spoke with the CI team and I believe the have fixed the issue that was causing this PR to fail. Could you fetch master, rebase atop it, and force push your changes to branch? That way we can see if the GitHub Actions are indeed fixed and a ticket should be generated to work on this PR. In the meantime, I will download your patch locally and have a look. Best, |
|
Hello @logan-connolly |
|
Hm ... |
|
Frustrating... you can see how flakiness in the actions is problematic for generating a ticket :/ But never mind that! I've got your patch locally and am running our test suite on it. When that passes, I'll write up a werk for the fix, push it up to our local infra to test, and then set someone for review. |
|
I have a question regarding the use of "NULL" as the default. Was there a special reason for using "NULL" here? I think it would make more sense to use "UNKNOWN" as the default since that is a supported state. It will in the end be truncated to "UNKN", but that is okay as it's a common abbreviation for the unknown state in Checkmk. |
In my opinion Just let me know if I should update the PR ... |
|
No, nothing to do on your side. I will just update the patch before submitting. Unfortunately, the component owner is on vacation until Friday. I plan to reach out to him regarding this last point. But in the meantime I will queue everything up so that we can just merge on Friday given his decision. Thank you again for your patience! |
|
Hi @zerwes, I've finally gotten the review and have merged in your contribution. It will take a bit to sync with the GitHub mirror. Once synced, the commit will be linked to this PR thread and the PR will be closed. You will be able to shortly see the associated werk under: https://checkmk.com/werk/18927 Thanks again for your contribution :) |
|
Thank you @logan-connolly for your support on this. |
Mail notifications failed when the following variables were undefined: - `PREVIOUSHOSTHARDSTATE` - `PREVIOUSSERVICEHARDSTATE` These variables now default to `UNKNOWN` when undefined. Closes: #857 JIRA-Ref: SUP-26054 Co-authored-by: Logan Connolly <logan.connolly@checkmk.com> Change-Id: I0809202931f353058ae64415d7bacfc0a792f251
see #852 (closed by accident while trying to rebase to force pushes on master using the github UI)
General information
Avoid failing notifications observed in the raw edition after upgrade to 2.4.x
Bug reports
In some cases notifications will fail in the raw edition due to some missing VARs with the error
jinja2.exceptions.UndefinedError: 'dict object' has no attribute 'PREVIOUSSERVICEHARDSTATE'
It seems the bug affects raw and enterprise installations (see #852 (comment))
More details can be found in https://forum.checkmk.com/t/after-upgrade-from-2-3-x-to-2-4-x-raw-sporadically-failing-notifications-with-jinja2-exceptions-undefinederror-dict-object-has-no-attribute-previousservicehardstate/55347
Proposed changes
The proposed changes just assure undefined values for the PREVIOUS[SERVICE|HOST]HARDSTATE in the jinja2 mail notification templates are set to some somehow sane defaults in order to avoid failing notifications.