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

API-24936-bgs-local-tracked-items #12194

Merged
merged 0 commits into from
Mar 23, 2023
Merged

Conversation

stiehlrod
Copy link
Contributor

Summary

As we have identified the Claims API latency issue to be related to the bgs-ext gem, we will need to move our BGS services for tracked items to Faraday as well.

Acceptance Criteria:

Move /findTrackedItems to Faraday

  • tam Dash

Related issue(s)

  • API-24936
  • Link to ticket created in va.gov-team repo
  • Link to previous change of the code/bug (if applicable)
  • Link to epic if not included in ticket

@stiehlrod stiehlrod added Lighthouse lighthouse claimsApi modules/claims_api labels Mar 21, 2023
@stiehlrod stiehlrod requested a review from a team as a code owner March 21, 2023 19:00
@stiehlrod stiehlrod self-assigned this Mar 21, 2023
@stiehlrod stiehlrod requested a review from a team as a code owner March 21, 2023 19:00
@va-vfs-bot va-vfs-bot temporarily deployed to API-24936-bgs-local-tracked-items/main/main March 21, 2023 19:04 Inactive
Comment on lines 148 to 150
{ claimId: id }.each do |k, v|
body.xpath("./*[local-name()='#{k}']")[0].content = v
end
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we could make this a helper method or something since every call will need it? That being said, I feel like code can be too dry...

raise ::Common::Exceptions::ResourceNotFound.new(detail: 'Claim tracked items not found')
end

raise
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably fine for now, but it'd be nice to have more error handling down the road as I'm sure we'll find more errors (I.E. connection timeout or a non-200 response).

acovrig
acovrig previously approved these changes Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants