andygrove opened a new pull request, #6089: URL: https://github.com/apache/datafusion-comet/pull/6089
## Which issue does this PR close? Closes #6088. ## Rationale for this change The versioning policy currently puts every `spark.comet.*` key inside the compatibility surface, with no carve-out. Read literally, that applies the full contract to configs declared with `.category(CATEGORY_TESTING)`: renaming one would need a `withAlternative` alias, changing a default would need a `spark.comet.legacy.*` escape hatch plus an upgrade guide entry, and removing one would need a deprecation cycle and a major release. Those keys are not a deployment surface. They exist so Comet's own suites, and contributors chasing a bug, can reach a state the rest of the code is not built to support — `spark.comet.scan.enabled` to isolate a planner problem, `spark.comet.explain.fallback.strict.enabled` to make a declined operator throw rather than quietly report itself, `spark.comet.testing.aggregate.partialMode.enabled` to run a partial aggregate without its final counterpart, and the on-heap family because Spark's own SQL suite needs on-heap mode. Guaranteeing them across releases would pin the internals they exist to expose, which is exactly what the rest of the policy leaves free to change. So the policy now says the `testing` category is exempt, and says what follows from that in both directions: maintainers owe those keys nothing, and contributors may not use the category as a way to ship a production knob without committing to it. ## What changes are included in this PR? Documentation only; no code changes. - `docs/source/about/versioning_policy.md`: a new `Testing Configurations Are Exempt` subsection under the compatibility surface, stating that a `testing` key's name, type, accepted values, default, and semantics may change in any release, with no alias, no legacy config, no deprecation cycle, and no upgrade guide entry — and that such a key must not be the only way to reach a behavior production users need. The covered/not-covered bullet lists are updated, and the three sections that previously implied the opposite now cross-reference it: patch releases (which said they add no configuration keys), behavior changes, and renames. - `docs/source/contributor-guide/config_conventions.md`: the `Categories` section now explains that the choice of category is what decides whether the key is covered, with guidance on when `testing` is the right answer. The rename checklist and the behavior-change section note the exemption. - `docs/source/user-guide/latest/configs.md`: a note above the Development & Testing Settings table so users see the exemption where they would set one of these. One thing worth a reviewer's attention: `internal()` is an orthogonal axis that the policy still does not address. Three keys are `internal()` but not in the `testing` category — `spark.comet.exec.broadcast.enabled`, `spark.comet.shuffle.jvm.spillThreshold`, and `spark.comet.shuffle.jvm.memoryWaitTimeout`. They are hidden from `configs.md` yet, under the policy as written before and after this PR, still covered. I left that alone rather than widen the change, but it is probably worth a second carve-out or a recategorization. ## How are these changes tested? Not applicable — documentation only. `prettier --check` passes on the three files, and every relative link and heading anchor added here resolves against an existing heading. -- 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]
