Skip to content

LG-15261 | Update AAMVA state maintenance windows#11705

Merged
n1zyy merged 3 commits intomainfrom
mattw/LG-15261_updated_aamva_mws
Jan 8, 2025
Merged

LG-15261 | Update AAMVA state maintenance windows#11705
n1zyy merged 3 commits intomainfrom
mattw/LG-15261_updated_aamva_mws

Conversation

@n1zyy
Copy link
Contributor

@n1zyy n1zyy commented Jan 3, 2025

🎫 Ticket

Link to the relevant ticket:
LG-15261

🛠 Summary of changes

LG-15261 had me reviewing the latest AAMVA DLDV user guide. I found that a substantial number of changes had taken place to maintenance windows.

📜 Testing Plan

I need to do some testing before this is merged. There are a few ambiguous cron expressions I want to test.

The latest version is linked from https://gitlab.login.gov/lg-teams/ada/remote-unsupervised/-/wikis/AAMVA/AAMVA-DLDV-User-Guide if you wish to view the original source of these.

n1zyy added 2 commits January 3, 2025 16:30
changelog: Internal, Identity Verification, Update maintenance windows for states
module Idv
class AamvaStateMaintenanceWindow
# All AAMVA maintenance windows are expressed in 'ET' (LG-14028),
# except Montana's which we converted here from MST to ET.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Montana's window has now been expressed in ET as well.

# "Daily system resets, normally at 4:45 am. to 5:15 am ET."
# "Daily, normally at 4:45 am. to 5:15 am ET."
{ cron: '45 4 * * *', duration_minutes: 30 },
# (Also "Sunday mornings but only seconds at a time.")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm leaving comments here for things that are documented but intentionally not implementing.

Comment on lines -41 to -43
'IN' => [
# Sunday morning maintenance from 6 am. to 10 am. ET.
{ cron: '0 6 * * Sun', duration_minutes: 4 * 60 },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forgot how the alphabet worked when I first added this. It reappears later.

# I'm modeling this as _every_ Wednesday since we're really
# answering "Should we expect a maintenance window right now?",
# and we don't block the user from anything.
{ cron: '0 21 * * Wed', duration_minutes: 3 * 60 },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can trim this comment down a bit, but want to capture that several of these are oddly worded and impossible to model as cron expressions.

I think it's important to discuss the motivation here: we are not turning users away and treating these as the state being unavailable. We are only modeling maintenance windows during which a lack of response from the DMV is not surprising.So where we don't have something more actionable, I'm erring on the side of being over-inclusive.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that makes sense

Copy link
Contributor

Choose a reason for hiding this comment

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

via some googling, and confirming for the Fugit gem, looks like there is a # syntax extension to cron Fugit supports

https://github.com/floraison/fugit?tab=readme-ov-file#the-hash-extension

and this claims to be a valid cron for "every 3rd wednesday" that matches that syntax

https://www.uptimia.com/cron/every-3rd-wednesday

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ooh, nice find!

That first link helps with #11705 (comment) where it turns out Sun#last is valid in Fugit.

The modulo extension supports every 3rd Wednesday, but whether this state means "Every 3rd Wednesday, starting January 1, 2019" is up in the air. I'll file a ticket to see if we can fish that information out of Cloudwatch logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I filed LG-15428 for digging into the "every third Wednesday" business, and opened https://github.com/18F/identity-idp/pull/11725/files for the other half of this.

Comment on lines +79 to +80
# "Monthly on Sunday, midnight to 10:00 am ET."
# (Okay, but _which_ Sunday?)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because this is 10 hours, I elected against modeling this one as weekly downtime.

'ND' => [
# Wednesday around 7:30 pm to 7:35 pm ET
{ cron: '30 19 * * Wed', duration_minutes: 5 },
# 3rd Sunday of month, 5 minutes anytime between midnight and noon.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not modeling 5 minutes of maintenance "anytime between midnight and noon"

Comment on lines +128 to +129
# MW FIXME: This wraps around, does Tue-Sun get parsed correctly?
{ cron: '0 2 * * Tue-Sun', duration_minutes: 1.25 * 60 },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cron starts on Sundays, so I may have to break this down to Tue-Sat and then Sun separately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It appears to work fine:

[6] pry(main)> f = Fugit.parse_cron('0 2 * * Tue-Sun')
=> #<Fugit::Cron:0x000000013206c128
 @cron_s=nil,
 @day_and=nil,
 @hours=[2],
 @minutes=[0],
 @monthdays=nil,
 @months=nil,
 @original="0 2 * * Tue-Sun",
 @seconds=[0],
 @timezone=nil,
 @weekdays=[[0], [2], [3], [4], [5], [6]],
 @zone=nil>

Comment on lines +157 to +159
# Last Sunday of every month from 11:00 pm Sunday to 2:00 am. Monday ET
{ cron: '0 23 * * Sun#4', duration: 3 * 60 },
{ cron: '0 23 * * Sun#5', duration: 3 * 60 },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is something cron can't model, so I just do the 4th and 5th.

What do you think of me doing a bit of date math and conditional assignment here? I've tended to view this as a static config file since it's initialized as a constant at application start, but there's no reason I can't do something like...

if there_are_5_sundays_this_month
  { cron: '0 23 * * Sun#5', duration: 3 * 60 }
else
  { cron: '0 23 * * Sun#4', duration: 3 * 60 }
end

I also briefly tried 0 23 25-31 * Sun, but it looks like that matches the 25th-31st of the month and also on every Sunday, rather than Sundays on the 25th-31st.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

# Downtime on weekends between 9 pm ET to 7 am ET.
{ cron: '0 21 * * Sat,Sun', duration_minutes: 10 * 60 },
# Saturday 9:00 pm. to Sunday 7:00 am. ET.
{ cron: '0 21 * * Sat', duration_minutes: 10 * 60 },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

☝️ This revised wording was helpful

{ cron: '0 5 * * *', duration_minutes: 30 },
# "Might not respond for short spells, daily between 7 pm and 8:30 pm." (not modeling this)
# Sunday morning maintenance 3:00 am. to 5 am. ET.
{ cron: '0 3 * * Sun', duration_minutes: 2 * 60 },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the same as it was before, but I moved it to match document order. (And changed to the 2 * 60 format for readability.)

@n1zyy n1zyy merged commit 0b303ab into main Jan 8, 2025
@n1zyy n1zyy deleted the mattw/LG-15261_updated_aamva_mws branch January 8, 2025 16:37
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.

4 participants