cloud-fan opened a new pull request, #56437:
URL: https://github.com/apache/spark/pull/56437
### What changes were proposed in this pull request?
This PR adds a new lightweight CI job `binding-policy` ("Config binding
policy exceptions check") to `build_and_test.yml`. The job diffs
`sql/hive/src/test/resources/conf/binding-policy-exceptions/configs-without-binding-policy-exceptions`
against the latest apache/spark base (using the same fork-sync mechanism as
the existing `buf` job) and fails if the PR adds any entries to the file, with
an error message pointing the author to
`.withBindingPolicy(ConfigBindingPolicy.SESSION/PERSISTED/NOT_APPLICABLE)`.
Removing entries from the file is still allowed (and encouraged).
The job runs on a bare runner in well under a minute, is gated on the
existing `lint` flag from the `precondition` job, only runs on forks (where PR
builds run, so the diff against apache/spark master is exactly the PR's
change), and skips cleanly on branches where the exceptions file does not exist.
### Why are the changes needed?
The exceptions file is a frozen list of pre-existing configs that were
created before the binding policy was introduced, and it should only ever
shrink. `SparkConfigBindingPolicySuite` forces every config to either declare a
binding policy or be listed in this file, so PR authors who add a new config
without a binding policy see a test failure and "fix" it by adding the config
to the exceptions file, which defeats the purpose of the enforcement (see
https://github.com/apache/spark/pull/56323#discussion_r3389669681 for a recent
example). A new config always has a valid policy choice, including
`NOT_APPLICABLE` for configs that do not affect the behavior of SQL
views/UDFs/procedures, so additions to the exceptions file are never justified.
Only a diff-based CI check can catch this, since to a regular test the grown
file looks consistent.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Simulated the check locally with the same script against three diffs: a
commit adding an entry to the exceptions file (check fails and lists the added
entries), a commit removing an entry (check passes), and no change to the file
(check passes). Also verified the workflow YAML parses.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code
This pull request and its description were written by Isaac.
--
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]
