refactor: replace default network client with OkHttp #1180
Conversation
… support setup - Updated build scripts to use `network-client-okhttp` by default where appropriate. - Revised README to simplify proxy setup instructions. - Adjusted integration workflows to correctly configure HTTP client preferences.
WalkthroughThe changes revert the default network client implementation from HttpURLConnection to OkHttp in the Java build configuration, update test workflows to specifically test the HttpURLConnection variant, modify Android dependencies to use OkHttp as the runtime client, and update documentation to recommend ProxyOptions for proxy support instead of OkHttp dependencies. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/integration-test.yml(5 hunks)README.md(1 hunks)android/build.gradle.kts(1 hunks)java/build.gradle.kts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: check-realtime-okhttp
- GitHub Check: check-rest-okhttp
- GitHub Check: check
- GitHub Check: check-liveobjects
- GitHub Check: check (29)
- GitHub Check: build
- GitHub Check: check-rest-httpurlconnection
- GitHub Check: check (19)
- GitHub Check: check-realtime-httpurlconnection
- GitHub Check: check (29)
- GitHub Check: check (21)
- GitHub Check: check (24)
🔇 Additional comments (3)
android/build.gradle.kts (1)
57-57: Make OkHttp explicit as Android's runtime network client.Switching to
runtimeOnlyscope for network-client-okhttp appropriately makes OkHttp the default HTTP client for Android users without exposing OkHttp internals at compile time. This aligns with the SDK's shift to OkHttp as the default across all platforms.README.md (1)
130-178: Clear and practical proxy setup documentation.The updated proxy guidance using
ProxyOptionsis cleaner and more user-friendly than requiring explicit OkHttp dependency management. The code example comprehensively covers both REST and Realtime clients with optional authentication, making it straightforward for users to implement proxy support.Verify that all classes and fields referenced in the example (
ProxyOptions,HttpAuth.Type.BASIC,HttpAuth.Type.DIGEST) exist and have the expected API..github/workflows/integration-test.yml (1)
11-92: Test matrix comprehensively covers both HTTP client variants; verify branch protection configuration.The updated workflow maintains test coverage for both OkHttp (default via okhttp jobs without the property) and HttpURLConnection (via httpurlconnection jobs with
-PhttpURLConnection). The renamed jobs clearly indicate what's being tested.Repository scan found no internal references to the old job names. However, verify that any branch protection rules on the main branch have been updated to reference the new job names (
check-rest-httpurlconnection,check-realtime-httpurlconnection,check-rest-okhttp,check-realtime-okhttp). Check this in GitHub repository settings under Branches → Branch protection rules and ensure required status checks reference the correct job names.
| if (findProperty("httpURLConnection") == null) { | ||
| runtimeOnly(project(":network-client-okhttp")) | ||
| } else { | ||
| runtimeOnly(project(":network-client-default")) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Breaking change: OkHttp is now the default network client.
The conditional logic now defaults to OkHttp when the httpURLConnection property is absent. Users who expect the previous HttpURLConnection-based default will need to explicitly pass -PhttpURLConnection to get that behavior. This is a significant behavioral change that should be clearly documented in release notes and may require user migration guidance.
Verify the following before release:
- A migration notice is added to CHANGELOG.md documenting this breaking change
- Users who specifically need HttpURLConnection behavior understand the
-PhttpURLConnectionflag requirement - Integration tests comprehensively validate both variants to catch compatibility issues early
- Any user-facing documentation is updated to mention OkHttp as the default HTTP client
Run this script to verify integration test coverage:
🏁 Script executed:
#!/bin/bash
# Check that integration tests cover both HTTP client variants
grep -n "httpURLConnection\|okhttp" .github/workflows/integration-test.yml | head -20Length of output: 187
🏁 Script executed:
#!/bin/bash
# Check for CHANGELOG entries about HTTP client changes
fd -e CHANGELOG -e CHANGELOG.md -e changelog.md -e HISTORY -e NEWS | head -10Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look for migration guides, release notes, or docs about HTTP client
fd -e md -e txt | xargs grep -l "httpURLConnection\|network.*client\|HTTP.*client" 2>/dev/null | grep -E "(CHANGELOG|README|MIGRATION|RELEASE|GUIDE|NEWS)" | head -20Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check if there's a README mentioning the default HTTP client
cat README.md 2>/dev/null | grep -A5 -B5 -i "http.*client\|okhttp\|network" | head -40Length of output: 831
🏁 Script executed:
#!/bin/bash
# Find all possible documentation files
find . -type f -name "*CHANGE*" -o -name "*RELEASE*" -o -name "*NEWS*" 2>/dev/null | head -20Length of output: 14
🏁 Script executed:
#!/bin/bash
# Check the build.gradle.kts file for comments explaining this logic
cat java/build.gradle.kts | head -40Length of output: 1013
🏁 Script executed:
#!/bin/bash
# Look for any docs directory with HTTP client documentation
find . -type d -name "docs" -o -name "doc" 2>/dev/null | xargs ls -la 2>/dev/null | head -30Length of output: 1796
🏁 Script executed:
#!/bin/bash
# Search CHANGELOG.md for HTTP client or httpURLConnection related entries
grep -n -A3 -B1 "httpURLConnection\|okhttp\|network.*client\|HTTP.*client" CHANGELOG.md | head -50Length of output: 0
Breaking change: OkHttp is now the default network client — add documentation before release.
The conditional logic defaults to OkHttp when the httpURLConnection property is absent. Integration tests properly cover both variants, but the breaking change lacks critical documentation:
- CHANGELOG.md: No entry documenting this behavioral change
- Migration guidance: No documentation for users who specifically need HttpURLConnection (must pass
-PhttpURLConnection) - User-facing docs: README.md doesn't mention the default HTTP client or how to switch between implementations
Users upgrading will unexpectedly receive OkHttp instead of HttpURLConnection without explicit migration instructions.
Add entries to CHANGELOG.md documenting the default change and add a section to README.md explaining HTTP client selection and the -PhttpURLConnection flag for users who need the legacy behavior.
🤖 Prompt for AI Agents
In java/build.gradle.kts around lines 25-29 the build now defaults to the OkHttp
network client when the -PhttpURLConnection property is absent; update docs
before release: add a CHANGELOG.md entry that clearly states the default changed
to OkHttp (include version, rationale, and note that HttpURLConnection remains
available via -PhttpURLConnection), add or update README.md with a new "HTTP
client selection" section that explains the default is OkHttp, shows the exact
flag to restore legacy HttpURLConnection (gradle -PhttpURLConnection or CI env
guidance), provides a short migration/compatibility note for users (e.g., known
behavior differences and testing advice), and ensure release notes / docs
reference any integration test coverage and how to run with the alternate client
for verification.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the network client configuration to use OkHttp as the default HTTP client instead of HttpURLConnection. The change simplifies proxy setup by making OkHttp (which has better proxy support) the default choice while still allowing users to opt-in to HttpURLConnection when needed.
- Inverted the build logic to make OkHttp the default network client, with HttpURLConnection as an opt-in via property flag
- Updated CI/CD workflows to test both HTTP client implementations separately
- Simplified README documentation by removing OkHttp-specific dependency instructions since it's now the default
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| java/build.gradle.kts | Inverted dependency logic to use network-client-okhttp by default, falling back to network-client-default (HttpURLConnection) when -PhttpURLConnection property is set |
| android/build.gradle.kts | Changed Android builds to unconditionally use network-client-okhttp as the runtime dependency |
| README.md | Removed OkHttp dependency installation instructions from proxy setup guide since OkHttp is now the default |
| .github/workflows/integration-test.yml | Renamed and restructured test jobs to explicitly test both HTTP client variants, with httpurlconnection tests using -PhttpURLConnection flag and okhttp tests running without flags |
Comments suppressed due to low confidence (1)
.github/workflows/integration-test.yml:57
- The artifact name for the Realtime test reports will conflict between the httpurlconnection and okhttp test jobs. When both
check-realtime-httpurlconnectionandcheck-realtime-okhttpjobs run, they will both try to upload artifacts with the namejava-build-reports-realtime, causing one to overwrite the other.
Consider making the artifact name unique by including the HTTP client variant, for example: java-build-reports-realtime-httpurlconnection.
- uses: actions/upload-artifact@v4
if: always()
with:
name: java-build-reports-realtime
path: java/build/reports/
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - uses: actions/upload-artifact@v4 | ||
| if: always() | ||
| with: | ||
| name: java-build-reports-rest | ||
| path: java/build/reports/ |
There was a problem hiding this comment.
The artifact names for the REST and Realtime test reports will conflict between the httpurlconnection and okhttp test jobs. When both check-rest-httpurlconnection and check-rest-okhttp jobs run, they will both try to upload artifacts with the name java-build-reports-rest, and similarly for the realtime jobs. This will cause one artifact to overwrite the other.
Consider making the artifact names unique by including the HTTP client variant in the name, for example:
java-build-reports-rest-httpurlconnectionandjava-build-reports-rest-okhttpjava-build-reports-realtime-httpurlconnectionandjava-build-reports-realtime-okhttp
network-client-okhttpby default where appropriate.Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.