awslabs / amazon-kinesis-video-streams-pic

Apache License 2.0

50 stars 47 forks source link

Adding a generic utility for exponential backoff retries #136

Closed kateyanurag closed 3 years ago

kateyanurag commented 3 years ago

Issue #, if available: N/A

Description of changes: Adding a utility for exponential backoff retries Testing: Added new unit tests

Travis CI See this PR - https://github.com/awslabs/amazon-kinesis-video-streams-pic/pull/137

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

kateyanurag commented 3 years ago

@hassanctech @MushMal pushed a commit addressing feedback

kateyanurag commented 3 years ago

@hassanctech @MushMal In the latest commit, I've addressed the following things -

Added checks to prevent overflow for power API
Changed power API name
Added check to ensure currentTime is always greater than lastRetryWaitTime to prevent any potential overflows
Stylistic changes to use CHK macro wherever applicable

Please take a look.

codecov-commenter commented 3 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (develop@6bbbe84). Click here to learn what that means. The diff coverage is n/a.

@@            Coverage Diff             @@
##             develop     #136   +/-   ##
==========================================
  Coverage           ?   87.20%           
==========================================
  Files              ?       52           
  Lines              ?     9260           
  Branches           ?        0           
==========================================
  Hits               ?     8075           
  Misses             ?     1185           
  Partials           ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6bbbe84...7095a66. Read the comment docs.

kateyanurag commented 3 years ago

There's are some flaky unrelated tests which are intermittently failing in travis. I'm closing this PR in favor of a "green" travis check in this PR.

hassanctech commented 3 years ago

There's are some flaky unrelated tests which are intermittently failing in travis. I'm closing this PR in favor of a "green" travis check in this PR.

It looks like this is the one that you merged, not the other one? Which I think is the right thing to do since this has the approval and I saw the CI succeeded on the merge, also the other PR is several commits behind (and is based off of the master branch instead of develop)