viirya opened a new pull request, #40:
URL: https://github.com/apache/spark-connect-gateway/pull/40
### 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.
--
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]
