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

feat: SDK-1568 expose okhttp client as a configuration in sdk core #857

Merged
merged 6 commits into from
Nov 26, 2024

Conversation

dtayeh
Copy link
Contributor

@dtayeh dtayeh commented Nov 19, 2024

Situation

Some users of the RAPID SDK are interested in getting more control over the underlying httpClient used by the SDK, such as tuning the dispatcher concurrency configs, or injecting interceptors/eventlisteners for monitoring purposes

Task

Enable users to inject their own okhttp client when configuring the SDK client.

Action

  • Client builder is modified into two builders, one with the shared configs, and another one extended with the timeouts configs.
  • All existing base clients extend the Client.BuilderExtension to maintain current behaviour
  • A new builder method that takes an okHttpClient as a configuration is added.
  • okHttpClient is added to the RAPID client configuration
  • Users can still configure timeouts on the default existing okhttp client

Testing

Manual testing by running the rapid examples covering test-cases:

  • Configuring timeouts on RAPID client
  • Configuring a custom okHttpClient, user is unable to configure timeouts outside the httpclient.
  • Configuring a client with the existing builder, users are able to set timeouts.

Results

Users are able to build and inject a custom okhttp client to be used by the SDK for executing requests.

@dtayeh dtayeh requested a review from a team as a code owner November 19, 2024 16:20
@dtayeh dtayeh force-pushed the dtayeh/SDK-1568 branch 2 times, most recently from 1e3eec7 to c8f5281 Compare November 20, 2024 15:03
Copy link
Member

@mohnoor94 mohnoor94 left a comment

Choose a reason for hiding this comment

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

After discussing the solution offline, we agreed to introduce a new builder that includes only the supported configurations, ensuring compatibility with the new okHttpClient settings and eliminating the need to throw exceptions.

@dtayeh dtayeh force-pushed the dtayeh/SDK-1568 branch 7 times, most recently from 1688f14 to c518e82 Compare November 25, 2024 10:25
mohnoor94
mohnoor94 previously approved these changes Nov 25, 2024
@dtayeh dtayeh force-pushed the dtayeh/SDK-1568 branch 2 times, most recently from d9c16a0 to 26c0df4 Compare November 26, 2024 13:02
mohnoor94
mohnoor94 previously approved these changes Nov 26, 2024
@dtayeh dtayeh force-pushed the dtayeh/SDK-1568 branch 3 times, most recently from 6fec878 to b0fbff5 Compare November 26, 2024 15:34
mohnoor94
mohnoor94 previously approved these changes Nov 26, 2024
@dtayeh dtayeh merged commit b36b9e6 into main Nov 26, 2024
4 checks passed
@dtayeh dtayeh deleted the dtayeh/SDK-1568 branch November 26, 2024 15:57
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