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

CV2-4233 add retry looper for alegre requests that intermittently fail #1786

Merged
merged 8 commits into from
Jan 31, 2024

Conversation

DGaffney
Copy link
Contributor

Description

When running via smooch, every once in a while, a request from Check-API to Alegre fails. There is every indication that this is some sort of intermittent, non-serious issue. When trying to reproduce them, even at large scales with lots of attempts to recreate the issue, Alegre responds correctly consistently. When looking at Alegre and Presto, neither show any indication that there is an issue. The response itself doesn't seem to be loud or fundamentally broken. For the time being, we're introducing a retry - and if that fails, we'll be much more confident that something more fundamental is happening that requires a deeper dive.

References: CV2-4233

How has this been tested?

I've added a direct test that should yield a proper null output after retrying multiple times, and ran a local script with side-effecting printouts to ensure it is doing what is intended.

Things to pay attention to during code review

Nothing really - fairly straightforward retry cycle

Checklist

  • I have performed a self-review of my own code
  • I have added unit and feature tests, if the PR implements a new feature or otherwise would benefit from additional testing
  • I have added regression tests, if the PR fixes a bug
  • I have added logging, exception reporting, and custom tracing with any additional information required for debugging
  • I considered secure coding practices when writing this code. Any security concerns are noted above.
  • I have commented my code in hard-to-understand areas, if any
  • I have made needed changes to the README
  • My changes generate no new warnings
  • If I added a third party module, I included a rationale for doing so and followed our current guidelines

Copy link
Contributor

@caiosba caiosba left a comment

Choose a reason for hiding this comment

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

A couple of suggestions and one request - also, tests must pass - good to go after that! :)

Copy link

codeclimate bot commented Jan 31, 2024

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

The test coverage on the diff in this pull request is 100.0% (100% is the threshold).

This pull request will bring the total coverage in the repository to 100.0% (0.0% change).

View more on Code Climate.

@DGaffney DGaffney requested a review from caiosba January 31, 2024 14:23
@DGaffney
Copy link
Contributor Author

@caiosba ran into some slightly weird speedbumps yesterday but back to full passage - lemme know if you need anything else before signoff!

@DGaffney DGaffney merged commit 6060da7 into develop Jan 31, 2024
8 checks passed
DGaffney added a commit that referenced this pull request Jan 31, 2024
#1786)

* CV2-4233 add retry looper for alegre requests that intermittently fail

* CV2-4233 fix fixture

* update test with right params

* change blah to proper type

* update function based on it being a hash that's actually being passed around

* refactors from PR

* add stub

* fix typo
@DGaffney DGaffney deleted the cv2-4233-retries-on-alegre branch February 1, 2024 14:18
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.

3 participants