Skip to content
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

Retry getting a rating profile on BGS::ShareError #15316

Merged
merged 5 commits into from
Sep 29, 2020

Conversation

leikkisa
Copy link
Contributor

@leikkisa leikkisa commented Sep 25, 2020

connects #15259

Description

Catches BGS::ShareError when fetching a rating profile, and re-tries fetching the rating profile with the other service.

Acceptance Criteria

  • When fetching rating profile on a PromulgatedRating, catch BGS Share errors with a similar message to above
  • Find the same rating using RatingAtIssue based on the profile date, and return it's rating profile

Testing Plan

  1. Automated test

@codeclimate
Copy link

codeclimate bot commented Sep 25, 2020

Code Climate has analyzed commit a6ca483 and detected 0 issues on this pull request.

View more on Code Climate.

@@ -65,9 +69,24 @@ def fetch_rating_profile
)
rescue Savon::Error
{}
rescue BGS::ShareError
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 decided to rescue the generic ShareError instead of the specific one originally listed in the AC because I figure anytime we get the ShareError, it's worth trying the other service as a backup.

Comment on lines +80 to +81
start_date: profile_date,
end_date: profile_date
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the start and end dates be the same?

Copy link
Contributor Author

@leikkisa leikkisa Sep 25, 2020

Choose a reason for hiding this comment

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

Yeah, with the regular service we can fetch a specific rating profile using the veteran's participant ID and profile date.

With the "back" service, we can fetch a range by date instead of a specific one. So this limits the results, and then another (just in case) matching is done using the profile date comparison method which includes the timestamp.

Copy link
Contributor

@yoomlam yoomlam left a comment

Choose a reason for hiding this comment

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

Left some comments. I'll let @tomas-nava review for approval.

Copy link
Contributor

@tomas-nava tomas-nava left a comment

Choose a reason for hiding this comment

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

approve!

It looks solid to me, but is there a way we can test in UAT or prod before making it widely available, since it changes the way we interact with BGS for a small number of cases?

@leikkisa
Copy link
Contributor Author

Okay, tested in UAT (not the ShareError specific case, but just that regular behavior is not breaking.

@leikkisa leikkisa added the Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master label Sep 29, 2020
@va-bot va-bot merged commit 3642b8e into master Sep 29, 2020
@va-bot va-bot deleted the sally/15259-handle-bgs-rating-profile-error branch September 29, 2020 17:30
leikkisa added a commit that referenced this pull request Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master Team: Foxtrot 🦊
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants