armeria icon indicating copy to clipboard operation
armeria copied to clipboard

Add builder for Backoff

Open kth496 opened this issue 1 year ago • 6 comments

Motivation:

  • #5483

Modifications:

  • Add Builder class for various backoff implementations.

Result:

  • Closes #5483
  • Will be able to use the builder class to create instances of BackOff.

kth496 avatar Mar 06 '24 11:03 kth496

@ikhoon Thank you for your review. I will fix it within a week.

kth496 avatar Mar 26 '24 01:03 kth496

@kth496 Looking forward to seeing more commits in this PR :heart_decoration:

trustin avatar May 08 '24 05:05 trustin

@kth496 Any updates?

trustin avatar Jun 06 '24 13:06 trustin

🔍 Build Scan® (commit: 75753598cfefe8d8c19f66d4187b32d6a2a7c7db)

Job name Status Build Scan®
build-ubicloud-standard-8-jdk-8 https://ge.armeria.dev/s/dm3hkglifq27s
build-ubicloud-standard-8-jdk-21-snapshot-blockhound https://ge.armeria.dev/s/7tt6wjdgqac36
build-ubicloud-standard-8-jdk-17-min-java-17-coverage https://ge.armeria.dev/s/guvjad7vttj56
build-ubicloud-standard-8-jdk-17-min-java-11 https://ge.armeria.dev/s/kqe4dqojq3it4
build-ubicloud-standard-8-jdk-17-leak https://ge.armeria.dev/s/7x7254bs2utd6
build-ubicloud-standard-8-jdk-11 ❌ (failure) https://ge.armeria.dev/s/zmddfogavz4l2
build-macos-12-jdk-21 https://ge.armeria.dev/s/c4t4v7dcuug6k

github-actions[bot] avatar Aug 09 '24 02:08 github-actions[bot]

@ikhoon @trustin Thanks for patience. I have improved the code based on your feedback:

  • Moved the nested classes into top-level classes
  • Moved the builder methods to the BackOff interface

Question: Is there a good way to make the jitter value configurable in the builder? Since jitter can be commonly set across multiple BackOff implementations, I'm considering whether to define a separate higher-level builder.

kth496 avatar Oct 03 '24 07:10 kth496

Codecov Report

Attention: Patch coverage is 83.87097% with 5 lines in your changes missing coverage. Please review.

Project coverage is 74.68%. Comparing base (9a29b39) to head (7575359). Report is 35 commits behind head on main.

Files with missing lines Patch % Lines
...corp/armeria/client/retry/FixedBackoffBuilder.java 0.00% 4 Missing :warning:
...ava/com/linecorp/armeria/client/retry/Backoff.java 75.00% 1 Missing :warning:
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #5488      +/-   ##
============================================
+ Coverage     74.66%   74.68%   +0.01%     
- Complexity    21681    21725      +44     
============================================
  Files          1896     1901       +5     
  Lines         80320    80414      +94     
  Branches      10548    10553       +5     
============================================
+ Hits          59974    60059      +85     
- Misses        15318    15324       +6     
- Partials       5028     5031       +3     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 03 '24 10:10 codecov[bot]