-
Notifications
You must be signed in to change notification settings - Fork 66
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
Mobile: Update lighthouse maintenance windows #15997
Conversation
I will pull this out of prod and fix tests when I get final confirmation that EVSS can be removed as a dependency to Lighthouse |
Letters isn't flipped
Ok learned a lot that's worth sharing. I removed a couple lighthouse dependencies because they aren't flipped yet. Found out that the new maintenance windows being tracked ( There are two big questions, one that we need to decide
In either case, Tom knows the process of adding new maintenance windows to the white list so if we solve automation, we should be able to breakdown lighthouse to however many maintenance windows we want |
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.
So if I understand this correctly, bgs, mpi, and vbms all turn off claims. I don't see an immunizations flag in the ticket. Does that exist? I wouldn't have expected [immunizations]
to work but I haven't looked at the underlying code in a long time.
Nice work figuring out who actually knows about maintenance windows. It's progress. Seems like this will be an ongoing project.
%i[lighthouse direct_deposit_benefits], | ||
%i[lighthouse disability_rating], | ||
%i[lighthouse letters_and_documents], | ||
%i[lighthouse immunizations], |
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.
My understanding is that lighthouse should take down immunizations. Did you get info from someone to the contrary?
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.
Ah right, forgot to push that change. I originally removed it because I was going to skip lighthouse altogether because currently pager duty is manually populated. I meant to add it back though
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.
So to be clear, if any of bgs, mpi, or vbms is down, then claims and immunizations are down? So we know that immunizations relies on all three of those?
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.
Fixing that, if it's wrong, is part of the larger ticket. This was just to decouple evss and lighthouse.
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.
Ya there will be another ticket to breakdown lighthouse
into a maintenance window for each endpoint we use. That process is all still being talked about but we'll be able to make this more specific
I just checked how the service graph works. |
…es' into mobile-maintenance-windows-updates
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.
Looks good. And I agree that given how often we have to feature flag in order to shift upstreams, it probably makes sense to refactor maintenance windows to respect feature flags.
Note: Delete the description statements, complete each step. None are optional, but can be justified as to why they cannot be completed as written. Provide known gaps to testing that may raise the risk of merging to production.
Summary
Updates the maintenance windows for mobile to respect new lighthouse PagerDuty services, along with further upstream services
Related issue(s)
department-of-veterans-affairs/va-mobile-app#8163
Testing done
Existing tests cover the functionality
Screenshots
Note: Optional
What areas of the site does it impact?
(Describe what parts of the site are impacted andifcode touched other areas)
Acceptance criteria
Requested Feedback
(OPTIONAL)What should the reviewers know in addition to the above. Is there anything specific you wish the reviewer to assist with. Do you have any concerns with this PR, why?