You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/12/27 00:58:13 UTC

[GitHub] [spark] HyukjinKwon opened a new pull request, #39224: [SPARK-41721][CONNECT][TESTS] Enable doctests in pyspark.sql.connect.catalog

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

   ### What changes were proposed in this pull request?
   
   This PR proposes to enable doctests in `pyspark.sql.connect.catalog` that is virtually the same as `pyspark.sql.catalog`.
   
   ### Why are the changes needed?
   
   To make sure on the PySpark compatibility and test coverage.
   
   ### Does this PR introduce _any_ user-facing change?
   
   No, test-only.
   
   ### How was this patch tested?
   
   CI in this PR should test this out.


-- 
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 #39224: [SPARK-41721][CONNECT][TESTS] Enable doctests in pyspark.sql.connect.catalog

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #39224:
URL: https://github.com/apache/spark/pull/39224#issuecomment-1365689704

   Merged to master.


-- 
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 #39224: [SPARK-41721][CONNECT][TESTS] Enable doctests in pyspark.sql.connect.catalog

Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #39224: [SPARK-41721][CONNECT][TESTS] Enable doctests in pyspark.sql.connect.catalog
URL: https://github.com/apache/spark/pull/39224


-- 
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] zhengruifeng commented on a diff in pull request #39224: [SPARK-41721][CONNECT][TESTS] Enable doctests in pyspark.sql.connect.catalog

Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on code in PR #39224:
URL: https://github.com/apache/spark/pull/39224#discussion_r1057439887


##########
python/pyspark/sql/catalog.py:
##########
@@ -273,15 +273,15 @@ def databaseExists(self, dbName: str) -> bool:
 
         >>> spark.catalog.databaseExists("test_new_database")
         False
-        >>> _ = spark.sql("CREATE DATABASE test_new_database")
+        >>> _ = spark.sql("CREATE DATABASE test_new_database").collect()

Review Comment:
   what about create a jira ticket for this (should change this back ideally)?



-- 
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 #39224: [SPARK-41721][CONNECT][TESTS] Enable doctests in pyspark.sql.connect.catalog

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #39224:
URL: https://github.com/apache/spark/pull/39224#discussion_r1057436437


##########
python/pyspark/sql/catalog.py:
##########
@@ -273,15 +273,15 @@ def databaseExists(self, dbName: str) -> bool:
 
         >>> spark.catalog.databaseExists("test_new_database")
         False
-        >>> _ = spark.sql("CREATE DATABASE test_new_database")
+        >>> _ = spark.sql("CREATE DATABASE test_new_database").collect()

Review Comment:
   Hmmm ... this is orthogonal issue but just to make sure we don't forget. I think we should make `sql` method in Spark Connect to be analyzed per every call. Otherwise, we can't make it compatible with the existing PySpark usage.
   
   I changes the existing doctest here for now to make the tests pass but should change this back ideally.



##########
python/pyspark/sql/catalog.py:
##########
@@ -273,15 +273,15 @@ def databaseExists(self, dbName: str) -> bool:
 
         >>> spark.catalog.databaseExists("test_new_database")
         False
-        >>> _ = spark.sql("CREATE DATABASE test_new_database")
+        >>> _ = spark.sql("CREATE DATABASE test_new_database").collect()

Review Comment:
   cc @hvanhovell @grundprinzip FYI



-- 
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 #39224: [SPARK-41721][CONNECT][TESTS] Enable doctests in pyspark.sql.connect.catalog

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #39224:
URL: https://github.com/apache/spark/pull/39224#discussion_r1057436437


##########
python/pyspark/sql/catalog.py:
##########
@@ -273,15 +273,15 @@ def databaseExists(self, dbName: str) -> bool:
 
         >>> spark.catalog.databaseExists("test_new_database")
         False
-        >>> _ = spark.sql("CREATE DATABASE test_new_database")
+        >>> _ = spark.sql("CREATE DATABASE test_new_database").collect()

Review Comment:
   Hmmm ... this is orthogonal issue but just to make sure we don't forget. I think we should make `sql` method in Spark Connect to be analyzed per every call when they execute such commands. Otherwise, we can't make it compatible with the existing PySpark usage.
   
   I change the existing doctest here for now to make the tests pass but should change this back ideally.



-- 
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 #39224: [SPARK-41721][CONNECT][TESTS] Enable doctests in pyspark.sql.connect.catalog

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #39224:
URL: https://github.com/apache/spark/pull/39224#discussion_r1057445504


##########
python/pyspark/sql/catalog.py:
##########
@@ -273,15 +273,15 @@ def databaseExists(self, dbName: str) -> bool:
 
         >>> spark.catalog.databaseExists("test_new_database")
         False
-        >>> _ = spark.sql("CREATE DATABASE test_new_database")
+        >>> _ = spark.sql("CREATE DATABASE test_new_database").collect()

Review Comment:
   👌 



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