You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "itholic (via GitHub)" <gi...@apache.org> on 2023/12/04 02:53:26 UTC
[PR] [SPARK-46230][PYTHON] Migrate `RetriesExceeded` and `RetryException` into PySpark error. [spark]
itholic opened a new pull request, #44147:
URL: https://github.com/apache/spark/pull/44147
### What changes were proposed in this pull request?
This PR proposes to migrate `RetriesExceeded` and `RetryException` into PySpark error.
### Why are the changes needed?
All errors defined from PySpark should be inherits `PySparkException` to keep the consistency of error messages generated from PySpark.
### Does this PR introduce _any_ user-facing change?
No, it's internal refactoring for better error handling.
### How was this patch tested?
The existing CI should pass.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
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
Re: [PR] [SPARK-46230][PYTHON] Migrate `RetriesExceeded` and `RetryException` into PySpark error. [spark]
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #44147:
URL: https://github.com/apache/spark/pull/44147#discussion_r1413416382
##########
python/pyspark/sql/connect/client/retries.py:
##########
@@ -248,13 +249,6 @@ def __iter__(self) -> Generator[AttemptManager, None, None]:
yield AttemptManager(self)
-class RetryException(Exception):
Review Comment:
Thanks for confirming! Will update the PR.
--
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
Re: [PR] [SPARK-46230][PYTHON] Migrate `RetriesExceeded` into PySpark error [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #44147: [SPARK-46230][PYTHON] Migrate `RetriesExceeded` into PySpark error
URL: https://github.com/apache/spark/pull/44147
--
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
Re: [PR] [SPARK-46230][PYTHON] Migrate `RetriesExceeded` into PySpark error. [spark]
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #44147:
URL: https://github.com/apache/spark/pull/44147#discussion_r1413419030
##########
python/pyspark/sql/connect/client/retries.py:
##########
@@ -248,13 +249,6 @@ def __iter__(self) -> Generator[AttemptManager, None, None]:
yield AttemptManager(self)
-class RetryException(Exception):
Review Comment:
Done reverting!
--
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
Re: [PR] [SPARK-46230][PYTHON] Migrate `RetriesExceeded` and `RetryException` into PySpark error. [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #44147:
URL: https://github.com/apache/spark/pull/44147#discussion_r1413412724
##########
python/pyspark/sql/connect/client/retries.py:
##########
@@ -248,13 +249,6 @@ def __iter__(self) -> Generator[AttemptManager, None, None]:
yield AttemptManager(self)
-class RetryException(Exception):
Review Comment:
`RetryException` is an internal error but I think `RetriesExceeded` is not.
--
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
Re: [PR] [SPARK-46230][PYTHON] Migrate `RetriesExceeded` into PySpark error [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #44147:
URL: https://github.com/apache/spark/pull/44147#issuecomment-1843520772
Thank you, @itholic and Hyukjin. 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
Re: [PR] [SPARK-46230][PYTHON] Migrate `RetriesExceeded` and `RetryException` into PySpark error. [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #44147:
URL: https://github.com/apache/spark/pull/44147#discussion_r1413358036
##########
python/pyspark/sql/connect/client/retries.py:
##########
@@ -248,13 +249,6 @@ def __iter__(self) -> Generator[AttemptManager, None, None]:
yield AttemptManager(self)
-class RetryException(Exception):
Review Comment:
I think this one is internal error IIRC. would need to double check.
--
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
Re: [PR] [SPARK-46230][PYTHON] Migrate `RetriesExceeded` and `RetryException` into PySpark error. [spark]
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on code in PR #44147:
URL: https://github.com/apache/spark/pull/44147#discussion_r1413392482
##########
python/pyspark/sql/connect/client/retries.py:
##########
@@ -248,13 +249,6 @@ def __iter__(self) -> Generator[AttemptManager, None, None]:
yield AttemptManager(self)
-class RetryException(Exception):
Review Comment:
I see. @cdkrot could you confirm if `RetryException` and `RetriesExceeded` are internal error or not? If they are internal error, I think we can just close this PR.
--
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
Re: [PR] [SPARK-46230][PYTHON] Migrate `RetriesExceeded` and `RetryException` into PySpark error. [spark]
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #44147:
URL: https://github.com/apache/spark/pull/44147#discussion_r1413413050
##########
python/pyspark/sql/connect/client/retries.py:
##########
@@ -248,13 +249,6 @@ def __iter__(self) -> Generator[AttemptManager, None, None]:
yield AttemptManager(self)
-class RetryException(Exception):
Review Comment:
Okay, I did a quick check. Let's only do it for `RetriesExceeded`
--
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
Re: [PR] [SPARK-46230][PYTHON] Migrate `RetriesExceeded` into PySpark error [spark]
Posted by "itholic (via GitHub)" <gi...@apache.org>.
itholic commented on PR #44147:
URL: https://github.com/apache/spark/pull/44147#issuecomment-1843857159
Thanks!
--
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