juliuszsompolski commented on code in PR #42212:
URL: https://github.com/apache/spark/pull/42212#discussion_r1277748268


##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/service/SparkConnectService.scala:
##########
@@ -161,6 +168,54 @@ class SparkConnectService(debug: Boolean)
         userId = request.getUserContext.getUserId,
         sessionId = request.getSessionId)
   }
+
+  // customizedMethodDesc returns a customized MethodDescriptor
+  // with updated request and response marshallers.
+  private def customizedMethodDesc(

Review Comment:
   nit: actually naming it methodWithCustomMarshallers will be self descriptive 
enough to likely not need the comment anymore
   (which I know I asked for comments a moment ago, but now as I understand it, 
I see better how to make it self-descriptive without comments)
   
   



##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/service/SparkConnectService.scala:
##########
@@ -161,6 +168,54 @@ class SparkConnectService(debug: Boolean)
         userId = request.getUserContext.getUserId,
         sessionId = request.getSessionId)
   }
+
+  // customizedMethodDesc returns a customized MethodDescriptor
+  // with updated request and response marshallers.
+  private def customizedMethodDesc(
+    methodDef: MethodDescriptor[MessageLite, MessageLite]
+  ): MethodDescriptor[MessageLite, MessageLite] = {
+    val recursionLimit =
+      SparkEnv.get.conf.get(CONNECT_GRPC_MARSHALLER_RECURSION_LIMIT)
+    val requestMarshaller =
+      ProtoLiteUtils.marshallerWithRecursionLimit(
+        methodDef
+          .getRequestMarshaller
+          .asInstanceOf[PrototypeMarshaller[MessageLite]]
+          .getMessagePrototype,
+        recursionLimit
+      )
+    val responseMarshaller =
+      ProtoLiteUtils.marshallerWithRecursionLimit(
+        methodDef
+          .getResponseMarshaller
+          .asInstanceOf[PrototypeMarshaller[MessageLite]]
+          .getMessagePrototype,
+        recursionLimit
+      )
+    methodDef.toBuilder(requestMarshaller, responseMarshaller).build()

Review Comment:
   nit: Would be more self-documenting as
   ```
   methodDef.toBuilder
     .setRequestMarshaller(requestMarshaller)
     .setResponseMarshaller(responseMarshaller)
     .build()
   ```
   instead of the shorthand method



-- 
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