-
Notifications
You must be signed in to change notification settings - Fork 166
LG-14028 | Model DMV maintenance windows in code #11142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e5e7ace
8e7c6f2
d377dab
a0d0b36
07d766b
076d174
3b0a67c
2e33b06
79f3cbd
741af4e
890e6f0
ef25ad4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,163 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Idv | ||
| class AamvaStateMaintenanceWindow | ||
| # _All_ AAMVA maintenance windows are expressed in 'ET' (LG-14028) | ||
| TZ = 'America/New_York' | ||
|
|
||
| MAINTENANCE_WINDOWS = { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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) |
||
| 'CA' => [ | ||
| # Daily, 4:00 - 5:30 am. ET. | ||
| { cron: '0 4 * * *', duration_minutes: 90 }, | ||
| # Monday, 1:00 - 1:45 am. ET | ||
| { cron: '0 1 * * Mon', duration_minutes: 45 }, | ||
| # Monday, 1:00 - 4:30 am. ET on 1st and 3rd Monday of month. | ||
| { cron: '0 1 * * Mon#1', duration_minutes: 3.5 * 60 }, | ||
| { cron: '0 1 * * Mon#3', duration_minutes: 3.5 * 60 }, | ||
| ], | ||
| 'CT' => [ | ||
| # Daily, 4:00 am. to 6:30 am. ET. | ||
| { cron: '0 4 * * *', duration_minutes: 90 }, | ||
| # Sunday 6:00 am. to 9:30 am. ET | ||
| { cron: '0 6 * * Mon', duration_minutes: 3.5 * 60 }, | ||
| ], | ||
| 'DC' => [ | ||
| # Daily, Midnight to 6 am. ET. | ||
| { cron: '0 0 * * *', duration_minutes: 6 * 60 }, | ||
| ], | ||
| 'DE' => [ | ||
| # Daily, Midnight to 5 am. ET. | ||
| { cron: '0 0 * * *', duration_minutes: 5 * 60 }, | ||
| ], | ||
| 'FL' => [ | ||
| # Sunday 7:00 am. to 12:00 pm. ET | ||
| { cron: '0 7 * * Sun', duration_minutes: 5 * 60 }, | ||
| ], | ||
| 'IA' => [ | ||
| # "Daily system resets, normally at 4:45 am. to 5:15 am ET." | ||
| { cron: '45 4 * * *', duration_minutes: 30 }, | ||
| ], | ||
| 'IN' => [ | ||
| # Sunday morning maintenance from 6 am. to 10 am. ET. | ||
| { cron: '0 6 * * Sun', duration_minutes: 4 * 60 }, | ||
| ], | ||
| 'IL' => [ | ||
| { cron: '30 2 * * *', duration_minutes: 2.5 * 60 }, # Daily, 2:30 am. to 5 am. ET. | ||
| ], | ||
| 'KY' => [ | ||
| # Daily maintenance from 2:50 am. to 6:40 am. ET | ||
| { cron: '50 2 * * *', duration_minutes: 230 }, | ||
| ], | ||
| 'MA' => [ | ||
| # Daily maintenance from 6 am. to 6:15 am. ET. | ||
| { cron: '0 6 * * *', duration_minutes: 15 }, | ||
| # Wednesday 7 am. to 7:30 am. ET. | ||
| { cron: '0 7 * * Wed', duration_minutes: 30 }, | ||
| # Saturday 10:00 pm. to Sunday 10:00 am | ||
| { cron: '0 22 * * Sat', duration_minutes: 12 * 60 }, | ||
| # First Friday of each month: 12 to 6 am. ET. | ||
| { cron: '0 0 * * Fri#1', duration_minutes: 6 * 60 }, | ||
| ], | ||
| 'MD' => [ | ||
| # Daily maintenance from 3 am. to 3:15 am. ET. | ||
| { cron: '0 3 * * *', duration_minutes: 15 }, | ||
| # Sunday maintenance may occur from 6 am. to 10 am. ET. | ||
| { cron: '0 6 * * Sun', duration_minutes: 4 * 60 }, | ||
| ], | ||
| 'MI' => [ | ||
| # Daily maintenance from 9 pm. to 9:15 pm. ET. | ||
| { cron: '0 21 * * *', duration_minutes: 15 }, | ||
| ], | ||
| 'MO' => [ | ||
| # Daily maintenance from 2 am. to 4:30 am. ... | ||
| { cron: '0 2 * * *', duration_minutes: 2.5 * 60 }, | ||
| # ... from 6:30 am to 6:45 am ... | ||
| { cron: '30 6 * * *', duration_minutes: 15 }, | ||
| # ... and 8:30 am. to 8:35 am ET. | ||
| { cron: '30 8 * * *', duration_minutes: 5 }, | ||
| # Sundays from 9 am. to 10:30 am. ET... | ||
| { cron: '0 9 * * Sun', duration_minutes: 90 }, | ||
| # ...and 5 am to 5:45 am ET on 2nd Sunday of month. | ||
| { cron: '0 5 * * Sun#2', duration_minutes: 45 }, | ||
| ], | ||
| 'NC' => [ | ||
| # Daily, Midnight to 7:00 am. ET. | ||
| { cron: '0 0 * * *', duration_minutes: 7 * 60 }, | ||
| # Sundays from 5am. till Noon | ||
| { cron: '0 5 * * Sun', duration_minutes: 7 * 60 }, | ||
| ], | ||
| # NM: "Sunday mornings." (not modeling; too vague) | ||
n1zyy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 'NY' => [ | ||
| # Sunday maintenance 8 pm. to 9 pm. ET. | ||
| { cron: '0 20 * * Sun', duration_minutes: 60 }, | ||
| ], | ||
| 'PA' => [ | ||
| # Sunday maintenance may occur, often between 5:30 am. & 7:00 am. ET | ||
| { cron: '30 5 * * Sun', duration_minutes: 90 }, | ||
| ], | ||
| 'SC' => [ | ||
| # Sunday maintenance from 7:00 pm. to 10:00 pm. ET. | ||
| { cron: '0 19 * * Sun', duration_minutes: 3 * 60 }, | ||
| ], | ||
| 'TX' => [ | ||
| # Downtime on weekends between 9 pm ET to 7 am ET. | ||
| { cron: '0 21 * * Sat,Sun', duration_minutes: 10 * 60 }, | ||
| ], | ||
| 'VA' => [ | ||
| # Sunday morning maintenance 3:00 am. to 5 am. ET. | ||
| { cron: '0 3 * * Sun', duration_minutes: 120 }, | ||
| # Daily maintenance from 5 am. to 5:30 am. | ||
| { cron: '0 5 * * *', duration_minutes: 30 }, | ||
| # "Might not respond for short spells, daily between 7 pm and 8:30 pm." (not modeling this) | ||
| ], | ||
| 'VT' => [ | ||
| # Daily maintenance from midnight to 5 am. ET. | ||
| { cron: '0 0 * * *', duration_minutes: 5 * 60 }, | ||
| ], | ||
| 'WA' => [ | ||
| # Maintenance from Saturday 9:45 pm. to Sunday 8:15 am. ET. | ||
| { cron: '45 21 * * Sat', duration_minutes: 10.5 * 60 }, | ||
| ], | ||
| 'WI' => [ | ||
| # Downtime on Tuesday – Saturday typically between 3 – 4 am ET. | ||
| { cron: '0 3 * * Tue-Sat', duration_minutes: 60 }, | ||
| # Downtime on Sunday from 6 – 10 am. ET. | ||
| { cron: '0 6 * * Sun', duration_minutes: 4 * 60 }, | ||
| ], | ||
| 'WV' => [ | ||
| # Occasional Sunday maintenance from 6:00 am. to noon ET. | ||
| { cron: '0 6 * * Sun', duration_minutes: 6 * 60 }, | ||
| ], | ||
| 'WY' => [ | ||
| # Daily, 2 am. to 5 am. ET. | ||
| { cron: '0 2 * * *', duration_minutes: 3 * 60 }, | ||
| ], | ||
| }.freeze | ||
|
|
||
| PARSED_MAINTENANCE_WINDOWS = MAINTENANCE_WINDOWS.transform_values do |windows| | ||
| Time.use_zone(TZ) do | ||
| windows.map do |window| | ||
| cron = Fugit.parse_cron(window[:cron]) | ||
| { cron: cron, duration_minutes: window[:duration_minutes] } | ||
| end | ||
| end | ||
| end.freeze | ||
|
|
||
| class << self | ||
| def in_maintenance_window?(state) | ||
| Time.use_zone(TZ) do | ||
| windows_for_state(state).any? { |window| window.cover?(Time.zone.now) } | ||
| end | ||
| end | ||
|
|
||
| def windows_for_state(state) | ||
| Time.use_zone(TZ) do | ||
| PARSED_MAINTENANCE_WINDOWS.fetch(state, []).map do |window| | ||
| previous = window[:cron].previous_time.to_t | ||
| (previous..(previous + window[:duration_minutes].minutes)) | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -8,7 +8,6 @@ class StateIdResult | |||||
|
|
||||||
| attr_reader :errors, | ||||||
| :exception, | ||||||
| :success, | ||||||
| :vendor_name, | ||||||
| :transaction_id, | ||||||
| :requested_attributes, | ||||||
|
|
@@ -21,7 +20,8 @@ def initialize( | |||||
| vendor_name: nil, | ||||||
| transaction_id: '', | ||||||
| requested_attributes: {}, | ||||||
| verified_attributes: [] | ||||||
| verified_attributes: [], | ||||||
| jurisdiction_in_maintenance_window: false | ||||||
| ) | ||||||
| @success = success | ||||||
| @errors = errors | ||||||
|
|
@@ -30,10 +30,11 @@ def initialize( | |||||
| @transaction_id = transaction_id | ||||||
| @requested_attributes = requested_attributes | ||||||
| @verified_attributes = verified_attributes | ||||||
| @jurisdiction_in_maintenance_window = jurisdiction_in_maintenance_window | ||||||
| end | ||||||
|
|
||||||
| def success? | ||||||
| success | ||||||
| !!@success | ||||||
| end | ||||||
|
|
||||||
| def timed_out? | ||||||
|
|
@@ -56,6 +57,10 @@ def mva_exception? | |||||
| mva_unavailable? || mva_system_error? || mva_timeout? | ||||||
| end | ||||||
|
|
||||||
| def jurisdiction_in_maintenance_window? | ||||||
| !!@jurisdiction_in_maintenance_window | ||||||
| end | ||||||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's more common to not define the so more like:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That was pretty much my instinct, but I followed the pattern of (I think my other instinct was to write none of this code and just make line 77 be
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've also used identity-idp/app/presenters/session_timeout_modal_presenter.rb Lines 18 to 19 in ea73d99
|
||||||
|
|
||||||
| def to_h | ||||||
| { | ||||||
| success: success?, | ||||||
|
|
@@ -67,6 +72,7 @@ def to_h | |||||
| transaction_id: transaction_id, | ||||||
| vendor_name: vendor_name, | ||||||
| verified_attributes: verified_attributes, | ||||||
| jurisdiction_in_maintenance_window: jurisdiction_in_maintenance_window?, | ||||||
| } | ||||||
| end | ||||||
| end | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| require 'rails_helper' | ||
|
|
||
| RSpec.describe Idv::AamvaStateMaintenanceWindow do | ||
| let(:tz) { 'America/New_York' } | ||
| let(:eastern_time) { ActiveSupport::TimeZone[tz] } | ||
|
|
||
| before do | ||
| travel_to eastern_time.parse('2024-06-02T00:00:00') | ||
| end | ||
|
|
||
| describe '#in_maintenance_window?' do | ||
| let(:state) { 'DC' } | ||
|
|
||
| subject { described_class.in_maintenance_window?(state) } | ||
|
|
||
| context 'for a state with a defined outage window' do | ||
| it 'is true during the maintenance window' do | ||
| travel_to(eastern_time.parse('June 2, 2024 at 1am')) do | ||
| expect(subject).to eq(true) | ||
| end | ||
| end | ||
|
|
||
| it 'is false outside of the maintenance window' do | ||
| travel_to(eastern_time.parse('June 2, 2024 at 8am')) do | ||
| expect(subject).to eq(false) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| context 'for a state without a defined outage window' do | ||
| let(:state) { 'LG' } | ||
|
|
||
| it 'returns false without an exception' do | ||
| expect(subject).to eq(false) | ||
| end | ||
| end | ||
| end | ||
|
|
||
| describe '.windows_for_state' do | ||
| subject { described_class.windows_for_state(state) } | ||
|
|
||
| context 'for a state with no entries' do | ||
| let(:state) { 'LG' } | ||
|
|
||
| it 'returns an empty array for a state with no entries' do | ||
| expect(subject).to eq([]) | ||
| end | ||
| end | ||
|
|
||
| context 'for a state with multiple overlapping windows' do | ||
| let(:state) { 'CA' } | ||
| let(:expected_windows) do | ||
| [ | ||
| eastern_time.parse('2024-06-01 04:00:00')..eastern_time.parse('2024-06-01 05:30:00'), | ||
| eastern_time.parse('2024-05-27 01:00:00')..eastern_time.parse('2024-05-27 01:45:00'), | ||
| eastern_time.parse('2024-05-06 01:00:00')..eastern_time.parse('2024-05-06 04:30:00'), | ||
| eastern_time.parse('2024-05-20 01:00:00')..eastern_time.parse('2024-05-20 04:30:00'), | ||
| ] | ||
| end | ||
|
|
||
| it 'returns all of them as ranges' do | ||
| Time.use_zone(tz) do | ||
| expect(subject).to eq(expected_windows) | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fugit is a dependency of goodjob, but make it a first-class citizen now that we're explicitly using it.