KTOR-9559 Add dnsResolver config to Apache5 engine#5571
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds optional DNS resolver support to the Apache5 engine: new nullable ChangesDNS resolver support
🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ktor-client/ktor-client-apache5/api/ktor-client-apache5.api`:
- Around line 16-23: The ABI dump lists getDnsResolver and setDnsResolver as
non-nullable, but the source property dnsResolver is nullable; regenerate the
Kotlin ABI for the module so the signatures reflect nullability (getDnsResolver
returns Lorg/apache/hc/client5/http/DnsResolver;? and setDnsResolver accepts a
nullable parameter). Run the project task to update the ABI dump (e.g.,
./gradlew :ktor-client-apache5:updateKotlinAbi) and commit the updated
ktor-client-apache5.api so getDnsResolver, setDnsResolver and the dnsResolver
property signatures match the source nullability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cce3911b-2f9f-4b66-87cd-c6195afa3e73
📒 Files selected for processing (4)
ktor-client/ktor-client-apache5/api/ktor-client-apache5.apiktor-client/ktor-client-apache5/jvm/src/io/ktor/client/engine/apache5/Apache5Engine.ktktor-client/ktor-client-apache5/jvm/src/io/ktor/client/engine/apache5/Apache5EngineConfig.ktktor-client/ktor-client-apache5/jvm/test/io/ktor/client/engine/apache5/Apache5EngineConfigTest.kt
dnsResolver config to Apache5 enginednsResolver config to Apache5 engine
7230205 to
5e87237
Compare
dnsResolver config to Apache5 enginednsResolver config to Apache5 engine
|
Thanks for the review! Updated to reference KTOR-9559 in the commit/title 😄 |
Subsystem
Client / Apache5 engine
Motivation
KTOR-9559 (umbrella: KTOR-462) / #1678 / #1924
Apache HttpClient 5 has a
DnsResolverinterface butApache5EngineConfigdoes not surface it as a first-class option. Users currently have to drop down toengine { configureConnectionManager { setDnsResolver(...) } }to inject a custom resolver.Solution
Add
Apache5EngineConfig.dnsResolver: DnsResolver?and apply it on thePoolingAsyncClientConnectionManagerBuilderbeforeconfigureConnectionManager. Defaults tonull→ Apache's default resolver is used as before.Follow-up to the OkHttp
dnsconfig PR (#5570); same per-engine pattern. The unified resolver abstraction (KTOR-462) remains out of scope.