This is an automated email from the ASF dual-hosted git repository.
HyukjinKwon pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark-connect-rust.git
The following commit(s) were added to refs/heads/master by this push:
new 7358e79 [SPARK-59076][INFRA] Fix all Rust compiler warnings and deny
warnings in CI
7358e79 is described below
commit 7358e7970814c992aa08937bc91aa7771c58711b
Author: Hyukjin Kwon <[email protected]>
AuthorDate: Fri Aug 28 16:50:20 2026 +0900
[SPARK-59076][INFRA] Fix all Rust compiler warnings and deny warnings in CI
### What changes were proposed in this pull request?
The Rust build emitted `dead_code` warnings that never failed CI. This
fixes all of them and turns warnings into **hard errors** so they cannot
regress.
**Fixes**
- `observation.rs`: remove the unused `dataframe` field and the
never-called `set_dataframe`/`set_metrics` methods (dead scaffolding — nothing
referenced them; `DataFrame.observe()` is a separate DataFrame method).
- `profiler.rs`: `ProfileResult.metadata` is populated from profiler
responses but not yet surfaced by `show()`/`dump()`; keep the collected data
and mark it `#[allow(dead_code)]` with a note rather than discarding it.
- golden test suites (catalog/ml/ml_extra/readwriter/window): the
`normalize_*`/`load_*`/`cint` helpers are copy-shared across the golden test
binaries and each binary uses only a subset, so add a file-level
`#![allow(dead_code)]` (the idiomatic treatment for shared cross-binary test
helpers; it correctly overrides `-D warnings` for that scope).
**Enforcement**
- `rust.yml`: workflow-level `RUSTFLAGS=-D warnings`.
- `build_python_connect.yml`: job-level `RUSTFLAGS=-D warnings` on the
*Rust checks* job (scoped so it does not gate the `pyspark-rs` extension build
in the parity job).
- Cargo caps dependency lints to `allow`, so this only fails on warnings in
our own workspace crates, never third-party deps.
### Why are the changes needed?
Warnings were accumulating unnoticed. A clean, warning-free build with
enforcement keeps it that way.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
CI (this PR), now with `-D warnings` gating the Rust build/test and Rust
checks jobs.
This pull request and its description were written by Isaac.
Closes #76 from HyukjinKwon/feature/deny-warnings.
Authored-by: Hyukjin Kwon <[email protected]>
Signed-off-by: Hyukjin Kwon <[email protected]>
---
.github/workflows/build_python_connect.yml | 5 +++++
.github/workflows/rust.yml | 6 ++++++
crates/spark-connect-macros/src/lib.rs | 6 ++++++
crates/spark-connect/src/observation.rs | 13 -------------
crates/spark-connect/src/profiler.rs | 5 ++++-
crates/spark-connect/tests/catalog_golden.rs | 2 ++
crates/spark-connect/tests/ml_extra_golden.rs | 2 ++
crates/spark-connect/tests/ml_golden.rs | 2 ++
crates/spark-connect/tests/readwriter_golden.rs | 2 ++
crates/spark-connect/tests/window_golden.rs | 2 ++
10 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/.github/workflows/build_python_connect.yml
b/.github/workflows/build_python_connect.yml
index 0296120..44a73a9 100644
--- a/.github/workflows/build_python_connect.yml
+++ b/.github/workflows/build_python_connect.yml
@@ -472,6 +472,11 @@ jobs:
rust-checks:
name: Rust checks
runs-on: ubuntu-latest
+ # Hard-fail on any compiler warning in our workspace crates (cargo caps
dependency
+ # lints to `allow`, so third-party deps are unaffected). Job-scoped on
purpose: the
+ # parity job above builds the pyspark-rs extension, which this must not
gate.
+ env:
+ RUSTFLAGS: "-D warnings"
steps:
- uses: actions/checkout@v4
- name: Install Rust toolchain
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
index 9a5d6a7..dd14bd4 100644
--- a/.github/workflows/rust.yml
+++ b/.github/workflows/rust.yml
@@ -30,6 +30,12 @@ on:
pull_request:
branches: [master]
+# Treat any compiler warning as a hard error across the whole Rust CI. Cargo
caps
+# lints for dependencies to `allow`, so this only fails on warnings in our own
+# workspace crates (dead code, unused imports, etc.), never on third-party
deps.
+env:
+ RUSTFLAGS: "-D warnings"
+
jobs:
rust-build-and-test:
name: Rust build and test
diff --git a/crates/spark-connect-macros/src/lib.rs
b/crates/spark-connect-macros/src/lib.rs
index c794d01..66a1720 100644
--- a/crates/spark-connect-macros/src/lib.rs
+++ b/crates/spark-connect-macros/src/lib.rs
@@ -276,6 +276,12 @@ fn expand(mut module: ItemMod) ->
Result<proc_macro2::TokenStream, syn::Error> {
let doc =
format!("UDF constructors generated by `#[spark_wasm_udf]` from module
`{mod_ident}`.");
Ok(quote! {
+ // Off wasm32 the exported UDF functions are called only from the
+ // `#[cfg(target_arch = "wasm32")]` wrappers injected above, so on the
host
+ // build they legitimately look unused - they exist to be compiled to
wasm.
+ // Allow dead_code there (not on wasm32, where the wrappers do use
them) so the
+ // macro does not force `-D warnings` failures on every generated UDF
module.
+ #[cfg_attr(not(target_arch = "wasm32"), allow(dead_code))]
#module
#[cfg(not(target_arch = "wasm32"))]
diff --git a/crates/spark-connect/src/observation.rs
b/crates/spark-connect/src/observation.rs
index 0d7aad4..d965eb6 100644
--- a/crates/spark-connect/src/observation.rs
+++ b/crates/spark-connect/src/observation.rs
@@ -2,7 +2,6 @@
//!
//! Mirroring `pyspark.sql.observation.Observation`.
-use crate::dataframe::DataFrame;
use std::collections::HashMap;
/// An Observation for collecting metrics from a DataFrame.
@@ -10,7 +9,6 @@ use std::collections::HashMap;
/// Mirrors `pyspark.sql.observation.Observation`.
pub struct Observation {
name: String,
- dataframe: Option<DataFrame>,
metrics: HashMap<String, String>,
}
@@ -19,7 +17,6 @@ impl Observation {
pub fn new(name: &str) -> Self {
Observation {
name: name.to_string(),
- dataframe: None,
metrics: HashMap::new(),
}
}
@@ -33,16 +30,6 @@ impl Observation {
pub fn get(&self) -> HashMap<String, String> {
self.metrics.clone()
}
-
- /// Internal method to set the DataFrame.
- pub(crate) fn set_dataframe(&mut self, df: DataFrame) {
- self.dataframe = Some(df);
- }
-
- /// Internal method to set metrics.
- pub(crate) fn set_metrics(&mut self, metrics: HashMap<String, String>) {
- self.metrics = metrics;
- }
}
#[cfg(test)]
diff --git a/crates/spark-connect/src/profiler.rs
b/crates/spark-connect/src/profiler.rs
index 9c3b6f0..579101a 100644
--- a/crates/spark-connect/src/profiler.rs
+++ b/crates/spark-connect/src/profiler.rs
@@ -51,7 +51,10 @@ fn format_literal(literal: &proto::expression::Literal) ->
String {
struct ProfileResult {
/// The accumulated profile data (typically a string representation).
pub data: String,
- /// Metadata about the profile (e.g., profile type, timestamp).
+ /// Metadata about the profile (e.g., profile type, timestamp). Collected
from the
+ /// profiler responses (see `collect_profiles`) for parity with pyspark;
retained for
+ /// a future `show()`/`dump()` that surfaces it, hence not read yet.
+ #[allow(dead_code)]
pub metadata: HashMap<String, String>,
}
diff --git a/crates/spark-connect/tests/catalog_golden.rs
b/crates/spark-connect/tests/catalog_golden.rs
index 460767a..a21f7bf 100644
--- a/crates/spark-connect/tests/catalog_golden.rs
+++ b/crates/spark-connect/tests/catalog_golden.rs
@@ -6,6 +6,8 @@
//! non-deterministic noise (common/origin, attribute plan_id) on BOTH sides,
then
//! require byte-equality. A required case that is missing or mismatches FAILS
the test.
+#![allow(dead_code)] // golden-test helpers are copy-shared across suites;
each binary uses a subset
+
use std::collections::HashMap;
use std::fs::File;
use std::io::{BufRead, BufReader};
diff --git a/crates/spark-connect/tests/ml_extra_golden.rs
b/crates/spark-connect/tests/ml_extra_golden.rs
index 2456a7a..fa7fa6d 100644
--- a/crates/spark-connect/tests/ml_extra_golden.rs
+++ b/crates/spark-connect/tests/ml_extra_golden.rs
@@ -3,6 +3,8 @@
//! Tests that Rust ML operator builders produce the exact same protobuf
//! structures as the reference PySpark ML Connect client for newer operators.
+#![allow(dead_code)] // golden-test helpers are copy-shared across suites;
each binary uses a subset
+
use std::collections::HashMap;
use std::fs::File;
use std::io::{BufRead, BufReader};
diff --git a/crates/spark-connect/tests/ml_golden.rs
b/crates/spark-connect/tests/ml_golden.rs
index 10f942a..20f85ae 100644
--- a/crates/spark-connect/tests/ml_golden.rs
+++ b/crates/spark-connect/tests/ml_golden.rs
@@ -3,6 +3,8 @@
//! Tests that Rust ML operator builders produce the exact same protobuf
//! the reference PySpark ML Connect client produces.
+#![allow(dead_code)] // golden-test helpers are copy-shared across suites;
each binary uses a subset
+
use std::collections::HashMap;
use std::fs::File;
use std::io::{BufRead, BufReader};
diff --git a/crates/spark-connect/tests/readwriter_golden.rs
b/crates/spark-connect/tests/readwriter_golden.rs
index de182b7..542dfab 100644
--- a/crates/spark-connect/tests/readwriter_golden.rs
+++ b/crates/spark-connect/tests/readwriter_golden.rs
@@ -3,6 +3,8 @@
//! Read and write operations must serialize to the exact same protobuf
//! the reference PySpark client produces.
+#![allow(dead_code)] // golden-test helpers are copy-shared across suites;
each binary uses a subset
+
use std::collections::HashMap;
use spark_connect::plan::LogicalPlan;
diff --git a/crates/spark-connect/tests/window_golden.rs
b/crates/spark-connect/tests/window_golden.rs
index 5528d5e..781048b 100644
--- a/crates/spark-connect/tests/window_golden.rs
+++ b/crates/spark-connect/tests/window_golden.rs
@@ -4,6 +4,8 @@
//! Goldens live in `tests/golden/window.jsonl` (captured by
//! `scripts/capture_window_golden.py`).
+#![allow(dead_code)] // golden-test helpers are copy-shared across suites;
each binary uses a subset
+
use std::collections::HashMap;
use std::fs::File;
use std::io::{BufRead, BufReader};
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
