andygrove opened a new pull request, #5056:
URL: https://github.com/apache/datafusion-comet/pull/5056
## Which issue does this PR close?
Part of #4082.
## Rationale for this change
The existing versioning policy documents the `0.x` series and explicitly
defers the post-1.0 rules: its "Road to 1.0" section says the stability
commitments for configuration keys and the protobuf plan format "will be
re-evaluated and documented as part of the `1.0.0` release."
`config_conventions.md` already links to the versioning policy for
alias-removal timing, so that cross-reference currently points at a rule that
does not exist yet.
Semantic versioning is usually described in terms of API signatures, which
does not fit Comet well. Comet is a plugin, not a library: the overwhelming
majority of users never touch a Comet type, they install it through Spark's
plugin system and configure it with `spark.comet.*` properties. Configuration
is the primary API surface, and the policy needs to say so before 1.0 ships
rather than after.
There is a genuine public Java and Scala API, but it is small, and today it
is nowhere written down. Leaving it implied invites both failure modes: users
treating an internal class as stable because it happens to be `public`, and
contributors breaking the S3 SPI without realizing a vendor jar compiled
against an earlier release depends on it. So this PR enumerates it in full.
Three positions in particular need to be settled up front, because they
determine what a major version bump means:
- **Behavior changes.** Following Apache Spark, a behavior change may ship
in a minor release provided users have a documented way to opt back out. That
escape hatch is a `spark.comet.legacy.*` key, mirroring `spark.sql.legacy.*`.
- **Correctness fixes.** Comet's goal is to return what Spark returns. When
a `Compatible` expression or operator does not, that is a bug, not a behavior
users are entitled to rely on. Treating those fixes as breaking changes would
make the major version number meaningless, since it would tick over for every
fix.
- **Spark version support.** Comet's supported Spark matrix tracks the
upstream project's maintenance windows. Tying it to semantic versioning would
force Comet major releases on a schedule set by another project, and would make
the major version say something about Spark rather than about Comet's own
compatibility.
## What changes are included in this PR?
Documentation only.
`docs/source/about/versioning_policy.md` is rewritten to be 1.0-forward. The
"Pre-1.0 Releases" body collapses to a note recording that the `0.x` series
remains unstable until `1.0.0` ships, and "Road to 1.0" is removed because this
document answers it. New content:
- **Comet's Compatibility Surface.** What the policy covers (`spark.comet.*`
keys, the plugin entry point, and query results for `Compatible` items) and
what it does not (the protobuf plan format, all Scala/Java/Rust types,
`EXPLAIN` output and plan shapes, metric names, log output, and performance
characteristics including which operators fall back to Spark).
- **What Each Version Component Means.** What justifies a major, minor, and
patch increment.
- **Behavior Changes and Legacy Configurations.** Defines a behavior change
as the same query over the same data with the same explicit configuration
producing a different result or error. Permits it in a minor release only when
a `spark.comet.legacy.*` boolean restores the previous behavior, the upgrade
guide gains an entry, and the release notes call it out. Legacy keys are
deprecated on arrival and removable only in a major release.
- **Correctness Fixes Are Not Breaking Changes.** With two riders:
maintainers may still add a legacy key for a fix with unusually wide blast
radius, and following an upstream Spark change is likewise not a Comet breaking
change.
- **Public Scala and Java API,** rewritten to enumerate the API rather than
gesture at it. The previous section promised a deprecation cycle for "public
classes and methods" without ever saying which ones those were. It is now an
explicit list in two parts, with everything outside it declared internal
whatever its access modifier says:
- **Class names referenced from configuration**, in a table:
`org.apache.spark.CometPlugin` (`spark.plugins`) and
`org.apache.comet.ExtendedExplainInfo` (`spark.sql.extendedExplainProviders`).
Users write these as config strings rather than compiling against them, so they
get the same treatment as a config key rename. Their internal structure carries
no guarantee.
- **The S3 credential provider SPI**: `CometS3CredentialProvider`,
`CometS3Credentials`, `CometS3CredentialContext`, and `CometS3AccessMode` in
`org.apache.comet.cloud.s3`. Vendors compile against these with `provided`
scope and ship a separate jar, so source *and* binary compatibility both
matter. Additive changes such as a new accessor on `CometS3CredentialContext`
are allowed in a minor release, matching the promise
`s3-credential-providers.md` already makes to vendors; anything that would
break an existing vendor jar, including adding an abstract method without a
default, requires a major. `CometS3CredentialDispatcher` is called out as
internal despite sharing the package, since it is the JNI entry point.
- **Spark version support placed outside semantic versioning,** folded into
the existing Apache Spark Version Support section. Adding or removing a Spark
minor, or a Scala binary version, is always a Comet minor release.
`docs/source/user-guide/latest/migration-guide.md` is new, and is added to
the user guide toctree as "Upgrading Comet". It mirrors Spark's SQL migration
guide: one section per Comet version listing behavior changes alongside the key
that reverts each. It is seeded with the page structure and a `1.0.0` section;
`0.x` history is not backfilled.
`docs/source/contributor-guide/config_conventions.md` gains "Changing the
Behavior of an Existing Config" beside the existing rename section, covering
the `spark.comet.legacy.*` recipe, the `CATEGORY_LEGACY` category, a checklist,
and the two cases that do not need a legacy key.
A couple of things deliberately left out:
- No code changes. `CATEGORY_LEGACY` is documented as the convention but not
declared in `CometConf.scala`; that belongs to whichever PR first adds a real
legacy config. Likewise, nothing annotates or otherwise marks the public API in
source: the enumeration lives only in the policy for now.
- The `@Unstable` annotation discussion is deleted. The old section said the
annotation "does not yet exist and will be introduced as the need arises"; with
the public API now enumerated explicitly, an exemption annotation for a
hypothetical unstable member is speculation we do not need to carry.
Open question for reviewers: `understanding-comet-plans.md` shows `import
org.apache.comet.ExtendedExplainInfo` in a Scala sample. The policy covers the
class *name* as a config value and treats the type itself as internal, with a
general note that documentation samples showing internal classes are debugging
aids rather than stability guarantees. If we would rather promise the type as
well, that is a small edit.
## How are these changes tested?
Documentation only, so there is no test to add. Verified locally:
- `sphinx -b html docs/source` builds successfully with no warnings
attributable to the added or modified pages, and the new upgrade guide is
reachable from the user guide navigation.
- All relative links in the touched files resolve, in both directions across
the versioning policy, the contributor guide, and the new upgrade guide.
- `prettier --check` passes on all touched markdown.
--
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]
