cdkrot commented on code in PR #43757:
URL: https://github.com/apache/spark/pull/43757#discussion_r1389511129


##########
connector/connect/common/src/main/scala/org/apache/spark/sql/connect/client/SparkConnectStubState.scala:
##########
@@ -28,15 +28,15 @@ import org.apache.spark.internal.Logging
 // logic.
 class SparkConnectStubState(
     channel: ManagedChannel,
-    val retryPolicy: GrpcRetryHandler.RetryPolicy)
+    val retryHandler: GrpcRetryHandler)
     extends Logging {
 
+  def this(channel: ManagedChannel, retryPolicies: 
Seq[GrpcRetryHandler.RetryPolicy]) =
+    this(channel, new GrpcRetryHandler(retryPolicies))
+
   // Responsible to convert the GRPC Status exceptions into Spark exceptions.
   lazy val exceptionConverter: GrpcExceptionConverter = new 
GrpcExceptionConverter(channel)
 
-  // Manages the retry handler logic used by the stubs.
-  lazy val retryHandler = new GrpcRetryHandler(retryPolicy)
-

Review Comment:
   I think I like it more like that, one retryHandler shared across everyone 
who wants it (why would you reinstantiate again)? If it would be like that, 
this Pr wouldn't have even need to touch this class.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to