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 6657bf2 [SPARK-59856] Record the provenance of the vendored Spark
Connect protos and add a sync script
6657bf2 is described below
commit 6657bf24c6e4b41aaed824ce479e14802b2684bf
Author: Liang-Chi Hsieh <[email protected]>
AuthorDate: Tue Sep 29 11:47:41 2026 -0700
[SPARK-59856] Record the provenance of the vendored Spark Connect protos
and add a sync script
### What changes were proposed in this pull request?
Record where the vendored Spark Connect protocol definitions came from, and
add a
script to check or re-sync them.
- **`proto/PROVENANCE.md`** — upstream project and path, the baseline
revision,
the licensing position, the two known deltas from upstream, and the
re-sync
procedure.
- **`dev/sync-protos.sh`** — compares `proto/spark/connect/` against
`apache/spark` and reports drift; overwrites only with `--sync`.
- **`README.md`** — points at both, and separates *regenerating the
bindings*
(already documented) from *updating the `.proto` files themselves* (was
not).
`proto/spark/connect/` itself is untouched by this PR.
### Why are the changes needed?
The eleven `.proto` files under `proto/spark/connect/` are copied verbatim
from
`apache/spark` and are the gateway's source of protocol truth —
`crates/genproto`
generates the Rust bindings from them on every build. Nothing in the repo
recorded
where they came from. README called them a "vendored read-only mirror of
upstream"
but named no revision, no upstream path, and no way to re-sync.
Three concrete costs:
1. **A release manager has no answer** to "where did these come from, and
under
what license" — a question that gets asked during an ASF release vote.
2. **Nobody can tell how far behind upstream the copy is**, or which
revision to
sync from, when Spark Connect's protocol moves.
3. **"Have these been edited locally since they were copied?"** could only
be
answered by archaeology.
Establishing the answer meant comparing all eleven files against upstream
tags, so
this records the result rather than leaving it to be redone.
**On the revision.** The copy is from the Spark 4.2 development line, not a
release: nine of the eleven files are byte-identical to `v4.2.0`, and the
files
themselves reference fields "deprecated since Spark 4.2+", but no release
tag
matches — `v4.2.0-rc1` through `rc5` were checked too. The doc records
`v4.2.0` as
the baseline to diff against and states explicitly that this is *not* a
claim the
files were taken from that tag. A version number that looks precise but is
wrong
would be worse than recording the uncertainty.
**The comparison surfaced drift that was not previously known.** The
vendored
protos are *behind* upstream:
| File | Missing relative to `v4.2.0` |
|---|---|
| `relations.proto` | the `NearestByJoin` message and its
`Relation.rel_type` entry |
| `pipelines.proto` | the `AutoCdcFlowDetails` message and its
`expressions.proto` import |
Both are pure upstream additions — nothing was edited locally. This PR
records
that and deliberately **does not** fix it: re-syncing moves the wire
protocol and
deserves review on its own, not folded into a documentation change.
The drift does not affect the gateway today. `crates/proxy` forwards each
request
onward whole and reads only session and identity metadata — nothing there
inspects
a plan's `rel_type` or walks the relation tree — so a request carrying one
of the
newer messages still routes and forwards correctly. I checked that in the
code
rather than assuming it, since it is the reason the drift is not urgent.
### Does this PR introduce _any_ user-facing change?
No. Documentation and a developer script; no protocol, code, or behaviour
change.
### How was this patch tested?
**The script was exercised, not just written.** Check mode agrees with the
manual
comparison that established the provenance in the first place:
```
$ dev/sync-protos.sh
vendored: proto/spark/connect
upstream: apache/spark v4.2.0
sql/connect/common/src/main/protobuf/spark/connect
DIFFERS pipelines.proto (+42/-0 lines vs upstream)
DIFFERS relations.proto (+31/-0 lines vs upstream)
identical: 9 differing: 2 only-upstream: 0 only-local: 0
```
The `+N/-0` shape is itself the evidence that the drift is one-directional
— we
are behind upstream, not forked from it.
`--ref v4.1.3` reports 6 identical / 5 differing, also matching the manual
comparison, which shows the `--ref` path works rather than the default being
hardcoded somewhere.
**`--sync` was tested on a throwaway copy of the tree** rather than in
place,
since it rewrites protocol files: it took that copy from 9/2 to 11/0, and
the
repo's own `proto/` was verified unchanged afterwards.
**Exit codes**: 1 for an unknown ref (with a message naming the ref), 2 for
an
unknown argument, 1 for `--ref` with no value, 0 for a clean check. Drift
itself
does not fail the check — the point of the default mode is to be runnable
any time
to see where things stand.
**Nothing regressed**: `proto/spark/` is unchanged, `cargo build -p
scg-genproto`
still compiles the bindings from those files, and the workspace is 204
passed, 0
failed, 10 ignored — the same as `main`.
One implementation note for review: the script fetches the upstream
*directory
listing* rather than iterating over the local files, so a `.proto` added
upstream
is reported as drift instead of being invisible. The reverse case —
vendored here,
absent upstream — is reported too.
### Was this patch authored or co-authored using generative AI tooling?
Yes, co-authored with Claude Code.
Closes #40 from viirya/proto-01-provenance.
Authored-by: Liang-Chi Hsieh <[email protected]>
Signed-off-by: Liang-Chi Hsieh <[email protected]>
---
README.md | 15 +++++-
dev/sync-protos.sh | 148 ++++++++++++++++++++++++++++++++++++++++++++++++++++
proto/PROVENANCE.md | 110 ++++++++++++++++++++++++++++++++++++++
3 files changed, 271 insertions(+), 2 deletions(-)
diff --git a/README.md b/README.md
index ed68108..f789474 100644
--- a/README.md
+++ b/README.md
@@ -414,8 +414,10 @@ crates/
observability/ # metrics, tracing, admin server
config/ # YAML config loader
genproto/ # tonic-generated bindings for spark.connect.*
-proto/spark/connect/
- *.proto # vendored read-only mirror of upstream
+proto/
+ PROVENANCE.md # where the vendored protos came from, and how to re-sync
+ spark/connect/
+ *.proto # vendored read-only mirror of upstream
deploy/examples/
spark-connect-server/ # K8s manifests (apache/spark-kubernetes-operator)
test/integration/
@@ -434,6 +436,15 @@ cargo build -p scg-genproto
`protoc` must be on `$PATH` (e.g. `brew install protobuf`).
+That regenerates the Rust bindings from the `.proto` files already in the tree.
+Updating the `.proto` files themselves against upstream Spark is a separate
+thing — see [proto/PROVENANCE.md](proto/PROVENANCE.md) for which revision they
+correspond to and how they drift, and `dev/sync-protos.sh` to check or re-sync:
+
+```bash
+dev/sync-protos.sh # report drift against the recorded baseline
+```
+
## What ships today
**Proxy core.** Streaming forward of every `SparkConnectService` RPC.
diff --git a/dev/sync-protos.sh b/dev/sync-protos.sh
new file mode 100755
index 0000000..0a487a5
--- /dev/null
+++ b/dev/sync-protos.sh
@@ -0,0 +1,148 @@
+#!/usr/bin/env bash
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# USAGE-BEGIN
+# Compare the vendored Spark Connect protocol definitions in proto/ against
+# apache/spark, or replace them with a given upstream revision.
+#
+# See proto/PROVENANCE.md for where these files came from and why the default
+# revision below is what it is.
+#
+# dev/sync-protos.sh # report drift against the default ref
+# dev/sync-protos.sh --ref v4.3.0 # report drift against another ref
+# dev/sync-protos.sh --sync # overwrite proto/ from the ref
+#
+# Checking is the default on purpose. Overwriting these files changes the
+# gateway's wire protocol, so it should be a deliberate act reviewed on its own
+# -- not a side effect of running a script to see where things stand.
+# USAGE-END
+
+set -euo pipefail
+
+# The revision proto/PROVENANCE.md records as the comparison baseline. Bump
this
+# together with that file whenever the vendored protos are re-synced.
+UPSTREAM_REF="v4.2.0"
+
+# Upstream location of the protocol definitions within apache/spark.
+UPSTREAM_DIR="sql/connect/common/src/main/protobuf/spark/connect"
+
+mode="check"
+while [ $# -gt 0 ]; do
+ case "$1" in
+ --sync) mode="sync"; shift ;;
+ --ref) UPSTREAM_REF="${2:?--ref needs a value}"; shift 2 ;;
+ -h|--help)
+ # Print the usage block above rather than keeping a second copy of it:
+ # everything between the markers, with the comment prefix stripped.
+ sed -n '/^# USAGE-BEGIN$/,/^# USAGE-END$/p' "$0" \
+ | grep -v -e '^# USAGE-BEGIN$' -e '^# USAGE-END$' \
+ | sed 's/^# \{0,1\}//'
+ exit 0 ;;
+ *) echo "unknown argument: $1" >&2; exit 2 ;;
+ esac
+done
+
+repo_root=$(cd "$(dirname "$0")/.." && pwd)
+local_dir="$repo_root/proto/spark/connect"
+base_url="https://raw.githubusercontent.com/apache/spark/${UPSTREAM_REF}/${UPSTREAM_DIR}";
+
+tmp=$(mktemp -d)
+trap 'rm -rf "$tmp"' EXIT
+
+echo "vendored: proto/spark/connect"
+echo "upstream: apache/spark ${UPSTREAM_REF} ${UPSTREAM_DIR}"
+echo
+
+# Fetch the upstream file list rather than iterating over what we already have,
+# so that a file added upstream shows up as drift instead of being invisible.
+if ! curl -fsS -m 60 \
+
"https://api.github.com/repos/apache/spark/contents/${UPSTREAM_DIR}?ref=${UPSTREAM_REF}";
\
+ > "$tmp/listing.json"; then
+ echo "could not list the upstream directory -- is ${UPSTREAM_REF} a real
ref?" >&2
+ exit 1
+fi
+
+python3 -c '
+import json, sys
+with open(sys.argv[1]) as fh:
+ entries = json.load(fh)
+if not isinstance(entries, list):
+ sys.exit("unexpected response from the GitHub contents API")
+names = sorted(e["name"] for e in entries
+ if e.get("type") == "file" and e["name"].endswith(".proto"))
+if not names:
+ sys.exit("no .proto files found upstream")
+print("\n".join(names))
+' "$tmp/listing.json" > "$tmp/upstream-names"
+
+same=0
+differ=0
+missing_local=0
+extra_local=0
+
+while read -r name; do
+ [ -n "$name" ] || continue
+ if ! curl -fsS -m 60 -o "$tmp/$name" "$base_url/$name"; then
+ echo " FETCH-FAILED $name" >&2
+ exit 1
+ fi
+ if [ ! -f "$local_dir/$name" ]; then
+ echo " ADDED-UPSTREAM $name (not vendored here)"
+ missing_local=$((missing_local + 1))
+ [ "$mode" = "sync" ] && cp "$tmp/$name" "$local_dir/$name"
+ continue
+ fi
+ if cmp -s "$local_dir/$name" "$tmp/$name"; then
+ same=$((same + 1))
+ else
+ added=$(diff "$local_dir/$name" "$tmp/$name" | grep -c '^>' || true)
+ removed=$(diff "$local_dir/$name" "$tmp/$name" | grep -c '^<' || true)
+ echo " DIFFERS $name (+${added}/-${removed} lines vs upstream)"
+ differ=$((differ + 1))
+ [ "$mode" = "sync" ] && cp "$tmp/$name" "$local_dir/$name"
+ fi
+done < "$tmp/upstream-names"
+
+# A file we vendor that upstream no longer has is drift too, in the other
+# direction -- most likely a rename or removal we have not picked up.
+for path in "$local_dir"/*.proto; do
+ name=$(basename "$path")
+ if ! grep -qxF "$name" "$tmp/upstream-names"; then
+ echo " NOT-UPSTREAM $name (vendored here, absent from ${UPSTREAM_REF})"
+ extra_local=$((extra_local + 1))
+ fi
+done
+
+echo
+echo "identical: ${same} differing: ${differ} only-upstream:
${missing_local} only-local: ${extra_local}"
+
+if [ "$mode" = "sync" ]; then
+ echo
+ echo "proto/ now matches ${UPSTREAM_REF}. Next:"
+ echo " 1. cargo build -p scg-genproto # the bindings are generated, not
committed"
+ echo " 2. cargo test --workspace"
+ echo " 3. update the revision in proto/PROVENANCE.md and UPSTREAM_REF above"
+ exit 0
+fi
+
+# Drift is the normal state between syncs, so report it without failing; the
+# whole point of the check mode is to be runnable at any time to see where the
+# vendored copy stands.
+if [ $((differ + missing_local + extra_local)) -gt 0 ]; then
+ echo "proto/ has drifted from ${UPSTREAM_REF}; see proto/PROVENANCE.md."
+ echo "Re-sync with: dev/sync-protos.sh --sync --ref ${UPSTREAM_REF}"
+fi
diff --git a/proto/PROVENANCE.md b/proto/PROVENANCE.md
new file mode 100644
index 0000000..7b8038d
--- /dev/null
+++ b/proto/PROVENANCE.md
@@ -0,0 +1,110 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one or more
+contributor license agreements. See the NOTICE file distributed with
+this work for additional information regarding copyright ownership.
+The ASF licenses this file to You under the Apache License, Version 2.0
+(the "License"); you may not use this file except in compliance with
+the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+# Provenance of the vendored Spark Connect protocol definitions
+
+The eleven `.proto` files under `spark/connect/` are **copied verbatim from
+[apache/spark](https://github.com/apache/spark)**. They are not written or
+maintained here, and they are not generated: they are checked-in schema source.
+
+| | |
+|---|---|
+| Upstream project | `apache/spark` |
+| Upstream path | `sql/connect/common/src/main/protobuf/spark/connect/` |
+| Baseline revision | `v4.2.0` (see *Which revision* below) |
+| Files vendored | all 11 `.proto` files in that directory |
+| License | Apache-2.0 — the same license as this project; each file keeps its
ASF header |
+
+The Rust bindings are generated from these files at build time by
+`crates/genproto/build.rs` (via `tonic-prost-build`) into `target/`, so no
+generated code is committed. `crates/genproto/build.rs` reads every `.proto` in
+the directory and fails if it is empty.
+
+## Which revision
+
+The vendored copy predates the `v4.2.0` release. It matches `v4.2.0` exactly
for
+9 of the 11 files, and the files themselves refer to fields "deprecated since
+Spark 4.2+", so it was taken from the Spark 4.2 development line — but from an
+untagged commit, not from a release. No `v4.2.0-rcN` tag matches either.
+
+`v4.2.0` is recorded as the baseline revision because it is the earliest
+*released* tag the vendored copy is consistent with, which makes it a
meaningful
+thing to diff against. It is not a claim that these files were taken from that
+tag.
+
+Two files differ from `v4.2.0`, in both cases because upstream has content the
+vendored copy does not:
+
+| File | Missing relative to `v4.2.0` |
+|---|---|
+| `spark/connect/relations.proto` | the `NearestByJoin` message and its
`Relation.rel_type` entry |
+| `spark/connect/pipelines.proto` | the `AutoCdcFlowDetails` message and its
`import "spark/connect/expressions.proto"` |
+
+Both are pure additions upstream: nothing in the vendored files was edited
+locally. In other words the vendored protos are slightly **behind** upstream,
not
+forked from it.
+
+This does not currently affect the gateway. It forwards each
+`SparkConnectService` request onward as a whole and only ever reads the
+session and identity metadata — nothing in `crates/proxy` inspects a plan's
+`rel_type` or walks the relation tree. So a request carrying one of these newer
+messages is still routed and forwarded correctly.
+
+What the gap does mean is that anything which needs to *inspect* one of those
+newer messages — a routing rule keyed on plan shape, say — would have to
re-sync
+first.
+
+## Checking and re-syncing
+
+`dev/sync-protos.sh` compares the vendored files against upstream and reports
+drift:
+
+```bash
+dev/sync-protos.sh # against the recorded baseline (v4.2.0)
+dev/sync-protos.sh --ref v4.3.0 # against some other upstream ref
+```
+
+It lists the upstream directory rather than iterating over the local files, so
a
+file **added** upstream is reported as drift instead of being silently missed.
A
+file vendored here but absent upstream is reported too.
+
+Re-syncing is a separate, deliberate step:
+
+```bash
+dev/sync-protos.sh --sync --ref v4.2.0
+```
+
+Checking is the default because overwriting these files changes the protocol
the
+gateway speaks. After a `--sync`:
+
+1. `cargo build -p scg-genproto` — regenerate the bindings and confirm they
compile.
+2. `cargo test --workspace` — the proxy's routing and session handling assume
the
+ message shapes.
+3. Update the **baseline revision** in the table above and `UPSTREAM_REF` in
+ `dev/sync-protos.sh` so they keep agreeing with each other.
+
+A re-sync is worth reviewing on its own rather than folding into an unrelated
+change, since it moves the wire protocol.
+
+## For a release
+
+These files are third-party content redistributed in the source release. They
+are Apache-2.0 licensed and retain their upstream ASF headers, so they need no
+separate entry in `LICENSE`, but the source release's `NOTICE`-side paperwork
+should account for them as sourced from `apache/spark`. That belongs with the
+`LICENSE-binary` / `NOTICE-binary` dependency-census work, which is still
+outstanding.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
