Skip to content

Conversation

@rjrudin
Copy link
Contributor

@rjrudin rjrudin commented Sep 19, 2025

This avoids hardcoding it in the actual client and let's us still see the results for the tests.

Also fixed a warning from Polaris about the interceptor.

@rjrudin rjrudin requested a review from anu3990 as a code owner September 19, 2025 19:30
Copilot AI review requested due to automatic review settings September 19, 2025 19:30
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves the RetryInterceptor configuration from being hardcoded in production code to being applied through test configuration. The key purpose is to enable retry functionality during tests while removing it from the production client code.

Key changes:

  • Removes hardcoded RetryInterceptor from production OkHttpUtil class
  • Adds RetryInterceptor configuration via DatabaseClientFactory configurators in test classes
  • Makes RetryInterceptor class and constructor public to enable external usage

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
Common.java Adds static block to configure RetryInterceptor for tests via DatabaseClientFactory
RetryInterceptor.java Changes class and constructor visibility to public; improves unreachable code handling
OkHttpUtil.java Removes hardcoded RetryInterceptor from production client builder
ConnectedRESTQA.java Adds static block to configure RetryInterceptor for functional tests

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


throw lastException;
// This should never be reached due to loop logic, but required for compilation
throw new IllegalStateException("Retry loop completed without returning or throwing - this should not happen");
Copy link

Copilot AI Sep 19, 2025

Choose a reason for hiding this comment

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

[nitpick] The comment and exception message are redundant. Consider simplifying to just throw new IllegalStateException("Unexpected end of retry loop"); since the comment already explains this should never be reached.

Suggested change
throw new IllegalStateException("Retry loop completed without returning or throwing - this should not happen");
throw new IllegalStateException("Unexpected end of retry loop");

Copilot uses AI. Check for mistakes.
This avoids hardcoding it in the actual client and let's us still see the results for the tests.

Also fixed a warning from Polaris about the interceptor.
@rjrudin rjrudin merged commit 9e70443 into develop Sep 22, 2025
2 checks passed
@rjrudin rjrudin deleted the feature/retry-fix branch September 22, 2025 14:05
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