You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "HyukjinKwon (via GitHub)" <gi...@apache.org> on 2023/02/01 07:38:04 UTC

[GitHub] [spark] HyukjinKwon opened a new pull request, #39840: [SPARK-42273][CONNECT][TESTS] Skip Spark Connect tests if dependencies are not installed

HyukjinKwon opened a new pull request, #39840:
URL: https://github.com/apache/spark/pull/39840

   ### What changes were proposed in this pull request?
   
   This PR proposes to skip doctests if the dependencies for Spark Connect are not installed.
   The skipping logic has to be added before the modules are loaded because it loads the module first when Python runs doctest. Otherwise, it fails as below:
   
   ```
   Traceback (most recent call last):
     File "/usr/local/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 197, in _run_module_as_main
       return _run_code(code, main_globals, None,
     File "/usr/local/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 87, in _run_code
       exec(code, run_globals)
     File "/.../spark/python/pyspark/sql/tests/connect/test_connect_basic.py", line 29, in <module>
       from pyspark.sql.connect.client import Retrying
     File "/.../spark/python/pyspark/sql/connect/__init__.py", line 21, in <module>
       from pyspark.sql.connect.dataframe import DataFrame  # noqa: F401
     File "/.../spark/python/pyspark/sql/connect/dataframe.py", line 50, in <module>
       import pyspark.sql.connect.plan as plan
     File "/.../spark/python/pyspark/sql/connect/plan.py", line 26, in <module>
       import pyspark.sql.connect.proto as proto
     File "/.../spark/python/pyspark/sql/connect/proto/__init__.py", line 18, in <module>
       from pyspark.sql.connect.proto.base_pb2_grpc import *
     File "/.../spark/python/pyspark/sql/connect/proto/base_pb2_grpc.py", line 19, in <module>
       import grpc
   ModuleNotFoundError: No module named 'grpc'
   ```
   
   ### Why are the changes needed?
   
   To make the tests pass without these optional dependencies.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No, dev-only.
   
   ### How was this patch tested?
   
   ```
   ./python/run-tests --module pyspark-connect -p 1
   ```


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #39840: [SPARK-42273][CONNECT][TESTS] Skip Spark Connect tests if dependencies are not installed

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #39840:
URL: https://github.com/apache/spark/pull/39840#issuecomment-1411593724

   cc @zhengruifeng 


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on a diff in pull request #39840: [SPARK-42273][CONNECT][TESTS] Skip Spark Connect tests if dependencies are not installed

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #39840:
URL: https://github.com/apache/spark/pull/39840#discussion_r1093961478


##########
python/pyspark/sql/connect/__init__.py:
##########
@@ -17,14 +17,26 @@
 
 """Currently Spark Connect is very experimental and the APIs to interact with
 Spark through this API are can be changed at any time without warning."""
+import sys
 
-from pyspark.sql.connect.dataframe import DataFrame  # noqa: F401
 from pyspark.sql.pandas.utils import (
     require_minimum_pandas_version,
     require_minimum_pyarrow_version,
     require_minimum_grpc_version,
 )
 
-require_minimum_pandas_version()
-require_minimum_pyarrow_version()
-require_minimum_grpc_version()
+
+def check_dependencies(mod_name, file_name):

Review Comment:
   ```suggestion
   def check_dependencies(mod_name: str, file_name: str) -> None:
   ```



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #39840: [SPARK-42273][CONNECT][TESTS] Skip Spark Connect tests if dependencies are not installed

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #39840:
URL: https://github.com/apache/spark/pull/39840#issuecomment-1413083142

   The last change was just to fix the type hint that dosen;t affect the test results.
   
   Merged to master and branch-3.4.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon closed pull request #39840: [SPARK-42273][CONNECT][TESTS] Skip Spark Connect tests if dependencies are not installed

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon closed pull request #39840: [SPARK-42273][CONNECT][TESTS] Skip Spark Connect tests if dependencies are not installed
URL: https://github.com/apache/spark/pull/39840


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on a diff in pull request #39840: [SPARK-42273][CONNECT][TESTS] Skip Spark Connect tests if dependencies are not installed

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #39840:
URL: https://github.com/apache/spark/pull/39840#discussion_r1093012669


##########
python/pyspark/sql/connect/__init__.py:
##########
@@ -17,14 +17,26 @@
 
 """Currently Spark Connect is very experimental and the APIs to interact with
 Spark through this API are can be changed at any time without warning."""
+import sys
 
-from pyspark.sql.connect.dataframe import DataFrame  # noqa: F401
 from pyspark.sql.pandas.utils import (
     require_minimum_pandas_version,
     require_minimum_pyarrow_version,
     require_minimum_grpc_version,
 )
 
-require_minimum_pandas_version()
-require_minimum_pyarrow_version()
-require_minimum_grpc_version()
+
+def check_dependencies(mod_name, file_name):

Review Comment:
   ```suggestion
   def check_dependencies(mod_name: str, file_name: str) -> None:
   ```



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org