You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "viirya (via GitHub)" <gi...@apache.org> on 2026/09/29 07:42:49 UTC
[PR] [SPARK-59856] Record the provenance of the vendored Spark Connect protos and add a sync script [spark-connect-gateway]
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59856] Record the provenance of the vendored Spark Connect protos and add a sync script [spark-connect-gateway]
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya closed pull request #40: [SPARK-59856] Record the provenance of the vendored Spark Connect protos and add a sync script
URL: https://github.com/apache/spark-connect-gateway/pull/40
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-59856] Record the provenance of the vendored Spark Connect protos and add a sync script [spark-connect-gateway]
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on PR #40:
URL: https://github.com/apache/spark-connect-gateway/pull/40#issuecomment-5896516863
Thanks @dongjoon-hyun !
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org