PR feedback

Author: 0marperez

runtime/protocol/http-client-engines/http-client-engine-okhttp/build.gradle.kts

@@ -36,7 +36,6 @@ kotlin {
 
         all {
             languageSettings.optIn("aws.smithy.kotlin.runtime.InternalApi")
-            languageSettings.optIn("okhttp3.ExperimentalOkHttpApi")
         }
     }
 }

runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/ConnectionIdleMonitor.kt

@@ -9,6 +9,7 @@ import kotlinx.coroutines.*
 import okhttp3.Call
 import okhttp3.Connection
 import okhttp3.ConnectionListener
+import okhttp3.ExperimentalOkHttpApi
 import okhttp3.internal.closeQuietly
 import okio.EOFException
 import okio.buffer
@@ -21,13 +22,13 @@ import kotlin.coroutines.coroutineContext
 import kotlin.time.Duration
 import kotlin.time.measureTime
 
+@OptIn(ExperimentalOkHttpApi::class)
 internal class ConnectionIdleMonitor(val pollInterval: Duration) : ConnectionListener() {
     private val monitorScope = CoroutineScope(Dispatchers.IO + SupervisorJob())
     private val monitors = ConcurrentHashMap<Connection, Job>()
 
     fun close(): Unit = runBlocking {
         monitors.values.forEach { it.cancelAndJoin() }
-        monitorScope.cancel()
     }
 
     private fun Call.callContext() =

runtime/protocol/http-client-engines/http-client-engine-okhttp/jvm/src/aws/smithy/kotlin/runtime/http/engine/okhttp/OkHttpEngine.kt

@@ -44,7 +44,7 @@ public class OkHttpEngine(
     }
 
     private val metrics = HttpClientMetrics(TELEMETRY_SCOPE, config.telemetryProvider)
-    private val connectionIdleMonitor = if (config.connectionIdlePollingInterval != null) ConnectionIdleMonitor(config.connectionIdlePollingInterval) else null
+    private val connectionIdleMonitor = config.connectionIdlePollingInterval?.let { ConnectionIdleMonitor(it) }
     private val client = config.buildClientWithConnectionListener(metrics, connectionIdleMonitor)
 
     override suspend fun roundTrip(context: ExecutionContext, request: HttpRequest): HttpCall {
@@ -100,12 +100,12 @@ private fun OkHttpEngineConfig.buildClientFromConfig(
         writeTimeout(config.socketWriteTimeout.toJavaDuration())
 
         // use our own pool configured with the timeout settings taken from config
-        val pool = ConnectionPool(
+        val pool = poolOverride ?: ConnectionPool(
             maxIdleConnections = 5, // The default from the no-arg ConnectionPool() constructor
             keepAliveDuration = config.connectionIdleTimeout.inWholeMilliseconds,
             TimeUnit.MILLISECONDS,
         )
-        connectionPool(poolOverride ?: pool)
+        connectionPool(pool)
 
         val dispatcher = Dispatcher().apply {
             maxRequests = config.maxConcurrency.toInt()
@@ -152,6 +152,7 @@ public fun OkHttpEngineConfig.buildClient(
  * Convert SDK version of HTTP configuration to OkHttp specific configuration and return the configured client
  */
 // Used by OkHttpEngine - OkHttp5 does have `connectionListener`
+@OptIn(ExperimentalOkHttpApi::class)
 private fun OkHttpEngineConfig.buildClientWithConnectionListener(
     metrics: HttpClientMetrics,
     connectionListener: ConnectionIdleMonitor?,