This is an automated email from the ASF dual-hosted git repository.
viirya pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/spark-connect-gateway.git
The following commit(s) were added to refs/heads/main by this push:
new f022ad9 [SPARK-59989] Pin the Rust toolchain and silence
double_must_use on async_trait traits
f022ad9 is described below
commit f022ad9bfa05cfd5f8b811965339b9c81a244d6b
Author: Liang-Chi Hsieh <[email protected]>
AuthorDate: Thu Oct 8 23:56:15 2026 -0700
[SPARK-59989] Pin the Rust toolchain and silence double_must_use on
async_trait traits
### What changes were proposed in this pull request?
Fix `main`'s failing CI, in two parts:
- **`rust-toolchain.toml`** — pin `channel` from `"stable"` to `"1.99.0"`,
with the
rationale and the steps for moving the pin recorded in the file.
- **`crates/routing/src/lib.rs`, `crates/auth/src/lib.rs`** — allow
`clippy::double_must_use` on the two `#[async_trait]` traits it fires on,
with a
comment explaining why.
### Why are the changes needed?
`main`'s CI has been red since 2026-10-04 on a commit that changed nothing
relevant:
```
2026-10-04 CI: failure <- main
2026-10-04 E2E: success
```
Because `rust-toolchain.toml` floated on `stable`, CI picked up the newly
released
Rust 1.99.0, whose clippy added the `double_must_use` lint.
The lint fires on every `#[async_trait]` trait here. `async_trait` expands
each
method into one returning a `#[must_use]` `Pin<Box<dyn Future>>`, and the
lint then
objects that the inner type — a `Result`, `Option` or `String` — is already
must-use. **The attribute belongs to the macro expansion, not to our
code**, so
there is nothing in these traits to restructure; allowing it is the right
answer
rather than a workaround.
**On pinning.** A floating channel means CI can turn red on a commit that
changed
nothing, at an unpredictable time, whenever upstream Rust ships a new lint
or
changes behaviour — which is exactly what happened here. Pinning makes the
build
reproducible between contributors and CI, and turns a toolchain upgrade
into a
reviewable change of its own rather than an event arriving on its own
schedule.
The cost is real and worth stating: Rust upgrades become manual. That seems
clearly
better than the alternative, but it is a trade-off rather than a free win.
MSRV is unaffected — `rust-version` stays at `1.82`, which is the minimum
this
compiles with, a separate thing from the version used to develop and gate.
### Does this PR introduce _any_ user-facing change?
No. Build configuration and two lint attributes; no behaviour change.
### How was this patch tested?
**Everything was verified on 1.99.0**, because that is what CI resolves to
— and
1.97.1, which a contributor may well have locally, does not reproduce the
failure
at all. That gap is the whole problem, so checking on the old version would
have
proved nothing.
- The failure **reproduces before the change**: 6 errors locally on 1.99.0,
matching CI.
- `cargo +1.99.0 clippy --workspace --all-targets -- -D warnings` — clean
after.
- `cargo +1.99.0 fmt --all --check` — clean.
- `cargo +1.99.0 test --workspace` — 204 passed, 0 failed, 10 ignored.
- A **bare `cargo`** now resolves to 1.99.0 through `rust-toolchain.toml`
and
passes, which confirms the pin takes effect for contributors rather than
only
when the version is named explicitly.
- **The pin takes effect in CI too**, which is worth showing explicitly
because
`.github/actions/setup-builder` still runs `rustup toolchain install
stable` and
the log's `Installing stable` line reads as though the pin were bypassed.
It is
not — rustup resolves the file ahead of the default, and says so:
```
info: note that the toolchain '1.99.0-x86_64-unknown-linux-gnu' is
currently in use
(overridden by '.../rust-toolchain.toml')
```
So `setup-builder`'s `rust-version: stable` input is now inert for version
selection; it still installs the components. Simplifying that input is
worth
doing, but as a separate change rather than widening this one.
**One detail about scope worth raising for review.** The CI log named only
`crates/routing/src/lib.rs`, because cargo stopped once that crate failed.
Reproducing locally showed `crates/auth/src/lib.rs:60` is affected too:
```
crates/auth/src/lib.rs:60
crates/routing/src/lib.rs:199
```
So fixing what the log reported would have left CI red on the next run with
a
different crate named. Both are fixed here — which is the argument for
reproducing
a toolchain failure locally rather than reading it off the log.
This unblocks #43 (SPARK-59973), which is red for this reason and not for
anything
in its own diff.
### Was this patch authored or co-authored using generative AI tooling?
Yes, co-authored with Claude Code.
Closes #44 from viirya/ci-11-pin-toolchain.
Authored-by: Liang-Chi Hsieh <[email protected]>
Signed-off-by: Liang-Chi Hsieh <[email protected]>
---
crates/auth/src/lib.rs | 5 +++++
crates/routing/src/lib.rs | 5 +++++
rust-toolchain.toml | 12 +++++++++++-
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/crates/auth/src/lib.rs b/crates/auth/src/lib.rs
index 282bf17..cfa16b5 100644
--- a/crates/auth/src/lib.rs
+++ b/crates/auth/src/lib.rs
@@ -57,6 +57,11 @@ use tonic::Status;
///
/// Implementations must be `Send + Sync + 'static` so the gateway can
/// hold them in an `Arc` and clone references into per-request futures.
+// `async_trait` expands each method into one returning a `#[must_use]`
+// `Pin<Box<dyn Future>>`, and clippy 1.99's double_must_use then objects that
the
+// inner type (a `Result`/`Option`/`String`) is already must-use. The
attribute is
+// the macro's, not ours, so there is nothing here to restructure.
+#[allow(clippy::double_must_use)]
#[async_trait]
pub trait Authenticator: Send + Sync + 'static {
/// Inspect `metadata`, validate any credential present, and return
diff --git a/crates/routing/src/lib.rs b/crates/routing/src/lib.rs
index f29d3e2..fb7c521 100644
--- a/crates/routing/src/lib.rs
+++ b/crates/routing/src/lib.rs
@@ -196,6 +196,11 @@ impl SelectionStrategy for RoundRobinStrategy {
/// network call. The in-memory impl wraps its sync work in async-fn
/// signatures with no `await` points, so callers pay only the
/// trait-object dispatch cost.
+// `async_trait` expands each method into one returning a `#[must_use]`
+// `Pin<Box<dyn Future>>`, and clippy 1.99's double_must_use then objects that
the
+// inner type (a `Result`/`Option`/`String`) is already must-use. The
attribute is
+// the macro's, not ours, so there is nothing here to restructure.
+#[allow(clippy::double_must_use)]
#[async_trait::async_trait]
pub trait AffinityStore: Send + Sync + 'static {
async fn lookup_session(&self, key: &SessionKey) -> Option<String>;
diff --git a/rust-toolchain.toml b/rust-toolchain.toml
index 92337f5..5b0c89c 100644
--- a/rust-toolchain.toml
+++ b/rust-toolchain.toml
@@ -14,5 +14,15 @@
# limitations under the License.
[toolchain]
-channel = "stable"
+# Pinned rather than "stable" on purpose. A floating channel means CI can turn
+# red on a commit that changed nothing, whenever upstream Rust ships a new lint
+# or changes behaviour -- and it did: clippy 1.99 added double_must_use, which
+# fires on every `#[async_trait]` trait in the workspace. A pinned version
makes
+# the build reproducible between contributors and CI, and turns a toolchain
+# upgrade into a reviewable change of its own.
+#
+# To move it: bump this, run `cargo clippy --workspace --all-targets -- -D
+# warnings` and `cargo test --workspace` locally on the new version, and fix
what
+# the new toolchain finds in the same PR.
+channel = "1.99.0"
components = ["rustfmt", "clippy"]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
