gaogaotiantian commented on code in PR #59019: URL: https://github.com/apache/spark/pull/59019#discussion_r4136974198
########## python/pyspark/tests/test_worker.py: ########## @@ -31,9 +34,37 @@ from py4j.protocol import Py4JJavaError from pyspark import SparkConf, SparkContext +from pyspark.serializers import read_int from pyspark.testing.utils import PySparkTestCase, QuietTest, ReusedPySparkTestCase, eventually +class WorkerProtocolTests(unittest.TestCase): Review Comment: This test is still too hacky. I don't believe the protocol test is a must - if all the udf tests passed, the protocol must work (or it'll hang). If we can't have a very nice way to test the feature (like reading the metrics from the driver/client side), we can skip it. One thing to confirm - the benchmark code still works with the protocol change. I remember that it simulates the socket communication between JVM and the worker process and our CI do not run it. -- 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]
