spark-test commented on code in PR #59947:
URL: https://github.com/apache/spark/pull/59947#discussion_r4426310101
##########
python/pyspark/sql/session.py:
##########
@@ -2275,7 +2275,12 @@ def addArtifacts(
----------
*path : tuple of str
Artifact's URIs to add. A native data source package (a
``.sparkpkg`` file) makes
- the native data sources it contains available to the session.
+ the native data sources it contains available to the session. It
fails for a
+ package that Spark cannot read, whose version Spark does not
support, or that has
+ no library for the platform of the driver; the session then does
not get the
+ package.
+ Without Spark Connect, the packages are added first, one at a
time, so a package
Review Comment:
**Nit (P3):** This sentence, which the Spark Connect session also uses as
its docstring (`connect/session.py`), limits the stop to the case without Spark
Connect, and the guide says that a call still adds the others "except in Python
without Spark Connect". The Python Spark Connect client stops at a rejected
package too. `SparkConnectClient.add_artifacts` sends one `AddArtifacts` stream
per path:
```python
for path in paths:
for attempt in self._retrying():
with attempt:
self._artifact_manager.add_artifacts(
path, pyfile=pyfile, archive=archive, file=file
)
```
The server returns the rejection as `INTERNAL`, which
`DefaultPolicy.can_retry` does not retry, so the first rejected path raises and
the later paths are never sent. For example, with Spark Connect,
`spark.addArtifacts("/tmp/bad.sparkpkg", "/tmp/good.jar")` raises for
`bad.sparkpkg`, and the session never gets `good.jar`. Only the Scala client
sends all the artifacts of a call in one stream, which
`SparkConnectAddArtifactsHandler` then collects.
Could the docstring and the guide say that in Python a rejected package
stops the rest of the call in both modes, and that with Spark Connect the paths
before it are added? Sending all the paths in one stream would also make the
current wording true, but it would change how every kind of artifact is added
from Python.
**Verification:**
- **Inspection:** The reworded docs match the implementation. Only
SparkRuntimeException rejections are collected within one addLocalArtifacts
call or one Spark Connect AddArtifacts stream, the Python Spark Connect client
sends one stream per path, and classic Python adds packages first, one at a
time.
- **Inspection:** No documentation of addArtifact or addArtifacts says that
a call continues after an I/O failure on a package file, or, in Python, after a
rejected package.
--
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]
