LG-14028 | Model DMV maintenance windows in code#11142
Conversation
Model known maintenance windows in code, so that we can set alerts to not fire if they're unreachable during a maintenance window. changelog: Internal, Alerting, Model DMV maintenance windows in code
| faraday (~> 2) | ||
| faraday-retry | ||
| foundation_emails | ||
| fugit |
There was a problem hiding this comment.
fugit is a dependency of goodjob, but make it a first-class citizen now that we're explicitly using it.
|
|
||
| module Idv | ||
| class AamvaStateMaintenanceWindow | ||
| MAINTENANCE_WINDOWS = { |
There was a problem hiding this comment.
See discussion in this Slack thread -- I initially had these in application.yml, but we decided they made sense in code. They will be changed very infrequently, and a deploy would have been needed anyway to update them.
There was a problem hiding this comment.
I think we should verify this assertion by tracking how often this file gets changed in the month or two after deploying it. If we have to edit them multiple times in a month, I think that's a little bit of evidence that they might be better off in config than in source code.
There was a problem hiding this comment.
I like that approach.
Realistically, these are currently from a doc from 2023 so I suspect it will not be changed often, unless we decide to move to modeling actual observed downtime vs. planned, recurring maintenance windows.
There was a problem hiding this comment.
Yeah, what will trigger an update here is getting a new user guide from AAMVA, which doesn't happen all that often (as @n1zyy said, current one is from 2023, and the rev log in there says that they had 3 updates in 2022)
spec/javascript/packages/document-capture/components/acuant-capture-spec.jsx
Show resolved
Hide resolved
|
|
||
| def jurisdiction_in_maintenance_window? | ||
| jurisdiction_in_maintenance_window | ||
| end |
There was a problem hiding this comment.
This follows the pattern, but also feels... superfluous? Opinions on just passing through the instance variable where this is used rather than adding a predicate method?
There was a problem hiding this comment.
It's more common to not define the attr_reader for booleans and just have the predicat method like this
so more like:
def jurisdiction_in_maintenance_window?
!!@jurisdiction_in_maintenance_window
end
There was a problem hiding this comment.
That was pretty much my instinct, but I followed the pattern of success (lines 38-40) that set up an attr_reader and then have a predicate method that just directly returns the attr_reader value.
(I think my other instinct was to write none of this code and just make line 77 be jurisdiction_in_maintenance_window: !!@jurisidiction_in_maintenance_window.)
There was a problem hiding this comment.
I've also used alias_method for this, though I get the impression that opinions are mixed w.r.t. aliasing.
identity-idp/app/presenters/session_timeout_modal_presenter.rb
Lines 18 to 19 in ea73d99
n1zyy
changed the title
6 participants
🎫 Ticket
Link to the relevant ticket:
LG-14028
🛠 Summary of changes
Many DMVs (or (insert other name for state entitities represented by AAMVA)) have recurring maintenance windows overnight. Most nights we get a bunch of alerts about high volumes of proofing errors.
We should not trigger an alert if we cannot reach a state during a known maintenance window. That will require an update to identity-devops to take effect.
Note that many of these maintenance windows are very broad and don't necessarily represent a guaranteed outage. It would be nice to eventually be smarter and not try to proof with AAMVA if we know there's an outage, but I think the data we have is too open-ended right now. This current PR is solely about allowing us to tweak our alerts, but there is potential room for doing something more advanced down the road.
📝 Still to do