Document CheckResult#previous_hard_state

[Al2Klimov]

No description provided.

[yhabteab]

Would be great to briefly explain (if any) the difference to Service#last_hard_state there as well. I think there's already quite a lot of confusion with all the (macro) runtime attributes and that doesn't make it any easier.

[julianbrost]

I'm decoupling this from #9723 as this PR doesn't really fix anything in that regard, it just adds some documentation for a missing attribute.

That issue claims that there is some discrepancy between the documentation and the implementation in regard to some state attribute (integer vs. string representation). Either that claim is correct, then this PR doesn't fix it or the claim is incorrect but instead probably the result of some confusion between object attributes and macros, then this PR doesn't fix it either and instead, we should check if this is properly documented or could be documented more clearly.

[julianbrost]

Originally, these macros were available containing state information:

• service.state
• service.state_id
• service.state_type
• service.last_state
• service.last_state_id
• service.last_state_type

Since #9171, there now additionally is:

• service.last_check_result.previous_hard_state

Which feels very out of place and was primarily added there due to some details of how notifications might be sent in HA zones (#9132 has some more details about that).

Now my first intuition would be to document it as such, i.e. some internal attribute, similar to how vars_before and vars_after already are. However, in #9723, it was actually suggested to use that value and it contains some information that seems to be unavailable to the user anywhere else.

So wouldn't it make more sense to add a corresponding macro to host/service, document it properly next to the other ones including how they interact (i.e. what is "last" and "previous" supposed to mean and so on) and document the one in the check result as "used internally, use that other one instead"?

Retrospectively, I'd say the one in check result shouldn't have been user-visible in the first place, but now it's there and we're probably stuck with it.

[Al2Klimov]

Please not yet another macro for the same thing! We're so close to finishing the milestone..

[julianbrost]

Having previous_hard_state in CheckResult feels more like an accidentally exposed implementation detail to me, do we really want to document it like something that's meant to be there?

We're so close to finishing the milestone..

I certainly don't want to rush any changes, all these state attributes/macros are already messy enough and any quick change might just make it worse in the end. So if we change something there, we should have a general concept (see also what I wrote in #9723 (comment)).

The question is: what's the best option for in the meantime?

• Document the value as if it was intended to be there. (Reason: it's there and it might bring some value to some users)
• Document it as internal (Reason: In my opinion, it shouldn't have been exposed there in the first place, so that would best describe it for me and provide some hint that things may change in the future).
• Maybe even don't document it at all for now.

I think some kind of documentation would primarily benefit users that find it by chance, either by looking at the check result docs or by noticing the attribute in some API response. I don't think someone would actually find it from the list of checkable attributes/macros (as I really wouldn't expect older information in the current check result, that's why I think this is attribute in the wrong place (when exposed to the user, for internal use, there are some reasons for that)).

[Al2Klimov]

• Maybe even don't document it at all for now.

Let’s do this and later make a plan.

[julianbrost]

Removed this PR from the milestone, so you might want to adjust #9760 accordingly.