OkHttpClient.Builder network pinning on Android #8376
Conversation
| @ExperimentalOkHttpApi | ||
| object NetworkSelection { | ||
| @RequiresApi(Build.VERSION_CODES.Q) | ||
| fun OkHttpClient.Builder.withNetwork(network: Network?): OkHttpClient.Builder { |
There was a problem hiding this comment.
I like this part of the API.
What happens if we attempt this in the main OkHttp module? We’d have a runtime dependency on Network, but you wouldn’t call this API if you don’t have it
There was a problem hiding this comment.
I am ok with either.
I guess ultimately we should choose either
a) we build android specific bits in main module, but you just don't use them if not needed.
b) we encourage every android app to include okhttp and okhttp-android and use those bits.
I'm not sure.
okhttp-android/src/main/kotlin/okhttp3/android/internal/NetworkPinInterceptor.kt
Outdated
Show resolved
Hide resolved
| callback: Callback, | ||
| ) | ||
|
|
||
| /** Returns a [Dns] that blocks until all async results are available. */ | ||
| open fun asBlocking(): Dns { |
There was a problem hiding this comment.
This all feels like a round-peg in a square-hole . . . building an async API and only ever using it in a blocking way.
I think we should build a regular Dns implementation for Android’s DNS stuff, and eject AsyncDns from our API until we’re ready to take advantage of it being async
There was a problem hiding this comment.
I was using it here, so you can have per-network Dns. I don't think that's possible without the async android resolver API.
There was a problem hiding this comment.
Ejecting the AsyncDns API for now. it's internal and moved to okhttp-android.
yschimke
changed the title
| @@ -27,33 +27,6 @@ public final class okhttp3/Address { | |||
| public final fun url ()Lokhttp3/HttpUrl; | |||
| } | |||
|
|
|||
| public abstract interface class okhttp3/AsyncDns { | |||
There was a problem hiding this comment.
@swankjesse no longer public API anywhere
Two features in 1 PR.
Can split up, but for discussion as it affects the API.
Cleanly pinning network on Android - #8286
Redo the Async DNS API, but internal for now - #8318