HyukjinKwon commented on PR #54:
URL:
https://github.com/apache/spark-connect-rust/pull/54#issuecomment-5419135801
<!-- ai-code-review -->
## Re-review at `462912c`
Fresh pass on the new head. The only change since `9a34c8a` is one test-only
commit (`462912c`, +43 lines in `e2e_coverage.rs::udf_builders_client_side`).
Nothing regressed. Two notes:
**The new test is another instance of finding B — and this one is a free
win.** `udf_builders_client_side` builds the
`applyInPandas`/`applyInArrow`/`applyInPandasWithState`/`transformWithState[InPandas]`/`cogroup`
applies and `mapInPandas`/`mapInArrow` plans and — as its own comment says —
never `collect()`s, so it needs **no server**. But it still opens with `if
!should_run() { return; }` (gated on `SPARK_REMOTE`, which no workflow sets),
so it runs nowhere in CI despite being a pure client-side plan-builder exercise
that *could*. Dropping the `should_run()` guard here (the builders return
`DataFrame`, no I/O) would turn it into genuine CI coverage of the
`group.rs`/`dataframe.rs` UDF request construction — which is the stated
target. As written it also asserts nothing (`let _ = gd.apply_in_pandas(udf())`
discards the lazy `DataFrame`), so even when run locally it's a no-panic smoke
test; a cheap `assert!` on the built plan's rel type would make it real.
**Still open from the previous pass** (this push didn't address them):
- **A (LOW):** `PyDataFrameWriterV2::option`/`options` (pyspark-rs
`dataframe.rs:1300`,`:1312`) still bypass `coerce_option_value` —
`writeTo(...).options(x=None)` sends `"None"`.
- **B (MEDIUM, test infra):** the `e2e_*` suite as a whole doesn't run in CI
(`SPARK_REMOTE` unset), and the `RightOuter`/`FullOuter`/`LeftSemi`/`LeftAnti`
joins + `local_relation`/`cached_remote_relation` still lack a server-free
serialization guard after the golden→behavioral swap.
No blocker. Verdict unchanged: good to merge once A/B are addressed or
consciously deferred.
This review was written by Isaac.
--
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]
