viirya opened a new pull request, #44:
URL: https://github.com/apache/spark-connect-gateway/pull/44
### 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 and CI rather
than
only when the version is named explicitly.
**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.
--
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]
