firasbouzazi opened a new pull request, #59242:
URL: https://github.com/apache/spark/pull/59242
### What changes were proposed in this pull request?
Fix the warning that `TorchDistributor._run_distributed_training` emits when
the log streaming server fails to start. The message had a stray comma, so the
error text was passed to `logger.warning` as an extra positional argument.
Formatting the record then raised `TypeError: not all arguments converted
during string formatting`. The warning now uses a single format string with the
exception as its argument (`"... error: %r.", e`).
### Why are the changes needed?
The fallback added in SPARK-44909 still works (training continues with the
port set to `-1`), but the warning that tells users why worker logs are missing
is broken:
- With default logging settings, users see a `--- Logging error ---`
traceback that looks like a crash instead of a one-line warning.
- With `logging.raiseExceptions = False`, the warning is silently dropped.
- `record.getMessage()` raises, so the warning cannot be asserted on.
### Does this PR introduce _any_ user-facing change?
Yes. When the log streaming server fails to start, users now get the
intended warning instead of a logging error traceback.
Before:
```
--- Logging error ---
Traceback (most recent call last):
...
TypeError: not all arguments converted during string formatting
Call stack:
File ".../pyspark/ml/torch/distributor.py", line 815, in
_run_distributed_training
self.logger.warning(
Message: 'Start torch distributor log streaming server failed, You cannot
receive logs sent from distributor workers, '
Arguments: ("error: OSError('Address already in use').",)
```
After:
```
Start torch distributor log streaming server failed, You cannot receive logs
sent from distributor workers, error: OSError('Address already in use').
```
### How was this patch tested?
Added `test_log_streaming_server_start_failure_warns` to
`TorchDistributorBaselineUnitTestsMixin`. It makes `LogStreamingServer.start`
raise and checks that training proceeds with the server disabled and that
exactly one warning is logged containing the error. The test fails with the
`TypeError` above before the fix and passes after it. The rest of
`TorchDistributorBaselineUnitTests` still passes.
```
python/run-tests --testnames 'pyspark.ml.torch.tests.test_distributor
TorchDistributorBaselineUnitTests'
```
### Was this patch authored or co-authored using generative AI tooling?
Co Authered with claude.
--
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]
