Skip to content

[infra] Require CI status checks to merge (main + branch-0.4) - #848

Open
vinishjail97 wants to merge 1 commit into
apache:mainfrom
vinishjail97:required-checks-main
Open

[infra] Require CI status checks to merge (main + branch-0.4)#848
vinishjail97 wants to merge 1 commit into
apache:mainfrom
vinishjail97:required-checks-main

Conversation

@vinishjail97

Copy link
Copy Markdown
Contributor

What

Makes CI green a hard requirement to merge. .asf.yaml branch protection is read only from the default branch (main) and previously declared no required_status_checks, so passing builds were informational, not gating — and branch-0.4 had no protection at all.

Required checks added:

  • main: Maven CI Build, License Check
  • branch-0.4: Maven CI Build, License Check, validate-spark-runtime-bundle (3.4.3), validate-spark-runtime-bundle (3.5.9)

(The Spark checks are branch-0.4-only — there is no spark-runtime module on main.)

Job rename

Maven CI Build and License Check both used a job named build, so both reported the same status context build and couldn't be required distinctly. Each now sets a unique job name, giving contexts Maven CI Build and License Check. A companion PR applies the same rename on branch-0.4 so those contexts exist there too.

Merge order

Merge after #843 lands on branch-0.4 (it adds the spark-runtime-validation workflow and its matrix). INFRA applies .asf.yaml wholesale on merge here, so if these rules land first, new branch-0.4 PRs would block on Spark contexts that aren't running yet. Also merge the branch-0.4 job-rename PR before/with this so the Maven CI Build / License Check contexts are produced there.

🤖 Generated with Claude Code

.asf.yaml branch protection is read only from the default branch, and
previously configured no required_status_checks, so green CI was not
gating merges. Add required checks:
  - main: Maven CI Build, License Check
  - branch-0.4: Maven CI Build, License Check, and both
    Spark Runtime Bundle Validation matrix legs (3.4.3, 3.5.9)

The two workflows that shared an ambiguous "build" job context now set
unique job names (Maven CI Build, License Check) so each can be required
distinctly. A matching branch-0.4 change renames the same jobs there.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vinishjail97 added a commit to vinishjail97/onetable that referenced this pull request Jul 20, 2026
Both workflows used a job named "build", so both reported the same
status-check context and could not be required distinctly. Set unique
job names so branch protection (see apache#848) can
require each on branch-0.4.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vinishjail97 added a commit that referenced this pull request Jul 21, 2026
…k 3.4 + 3.5) (#843)

* [836] xtable-spark-runtime: thin drop-in Spark bundle (Hudi 0.x)

Ports the xtable-spark-runtime packaging slice onto the Hudi 0.x line
(main-hudi-0x, Hudi 0.14 / Spark 3.4). New thin, relocated bundle that
runs an incremental XTable sync inside a Spark job; engines are provided
by the cluster, never bundled.

Module:
- xtable-spark-runtime_${scala.binary.version}: xtable-core compile;
  Spark/Hadoop and the engines (Hudi/Iceberg/Delta) provided. Curated
  shade allowlist (xtable modules + guava/protobuf/commons-cli relocated);
  avro/parquet/jackson NOT relocated (exchanged with the engines). Thin
  ~3.7 MB bundle.
- XTableSparkSync: standalone spark-submit entry point (Apache Commons CLI).
- XTableSyncService / TableSyncSpec: build an INCREMENTAL ConversionConfig
  and run ConversionController.sync; target metadata path = source data
  path (required by Hudi; Iceberg data lives under <basePath>/data).

Hudi 0.14 specifics (vs the Hudi 1.x variant on main):
- No hudi-hadoop-common (that split is 1.x); hudi-common provides FSUtils.
- Engine classpath uses the hudi-spark bundle (its regenerated Avro model
  classes link on Avro 1.12, which Iceberg 1.9.2 requires) plus
  hudi-java-client for the Hudi target's Java write client, with the raw
  hudi-common excluded so the bundle's clean DecimalWrapper wins.

ConversionTargetFactory: make ServiceLoader discovery resilient so a
subset of engines works when others are absent (warn + skip on
LinkageError/ServiceConfigurationError; name-based Delta-Kernel check).

ITXTableSparkRuntimeBundle: spark-submits the shaded jar for one case per
direction across Hudi, Iceberg and Delta (source and target), engines on
a flat classpath, asserting data-equivalence over comparable scalar
columns. Requires a Spark 3.4 SPARK_HOME; skipped otherwise.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* [836] xtable-spark-runtime: address review feedback (packaging, licensing, RFC)

Publish the shaded jar as the MAIN artifact with a dependency-reduced POM
(shadedArtifactAttached=false, createDependencyReducedPom=true), matching
iceberg-spark-runtime / hudi-spark-bundle. Resolving the coordinate via a
Maven dependency or --packages now yields the relocated bundle and pulls no
un-relocated transitive deps; --jars is equivalent. The thin main jar +
separate -bundle classifier (which re-introduced the cluster guava clash)
is gone. IT findBundleJar() now picks the shaded main jar.

Pass release/scripts/validate_shaded_license_coverage.sh (the existing
allowlist gate): the shade <includes> must equal the runtime dependency
tree, so jackson / scala-library / log4j-1.2-api - which every Spark
runtime supplies - are declared provided (dropped from the tree, kept off
the shaded jar) and excluded from the IT's flat engine classpath so Spark's
own copies win. avro/parquet stay on the flat classpath (the engine's newer
avro must win over Spark 3.4's).

Bundled-dependency licensing: add META-INF/LICENSE-bundled and
NOTICE-bundled (wired via IncludeResourceTransformer) attributing the only
bundled third-party - guava's closure (Apache-2.0) and commons-cli
(Apache-2.0), plus checker-qual (MIT). Remove the dead protobuf-java
allowlist entry and relocation: protobuf resolves as provided and was never
bundled.

RFC-3: scope v1 to the CLI (XTableSparkSync); mark the config-only
XTableSyncListener on-ramp (and XTableSparkConfig / PlanTargetResolver) as a
deferred follow-up. Fix the activation example to the shipped CLI and
document the required engine Avro version + flat-classpath placement.

XTableSyncService: normalize sourceFormat with Locale.ROOT.

Root pom: exclude the shade-generated dependency-reduced-pom.xml from
spotless (no license header; CI runs clean install).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* [836] xtable-spark-runtime: trim duplicated pom comments

Consolidate repeated rationale in the bundle pom (no functional change):
- Tell the hudi-common DecimalWrapper / Avro-1.8-1.9 story once (on the
  hudi-spark bundle dep); the hudi-java-client exclusion just points to it.
- State "engine Avro must win on a flat classpath" once (engine-classpath
  plugin); the avro dep and excludeGroupIds comments reference it.
- Explain jackson/scala-library are Spark-supplied once.
- Fix a stale "Spark 3.5" reference to Spark 3.4 (this is the Hudi 0.x line).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* [836] ci: trigger Maven CI + License Check on main-hudi-0x

Add main-hudi-0x to the push and pull_request branch filters of both
workflows (same change as #841) so CI runs on PRs targeting the Hudi 0.x
release line, including this one. A pull_request workflow's branch filter
takes effect from the PR's own branch, so this makes CI fire on #843.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Address review feedback: guard ServiceLoader.hasNext() and drain spark-submit stdout off-thread

- ConversionTargetFactory: hasNext() resolves provider classes lazily and can
  throw ServiceConfigurationError too, so move it inside the existing
  ServiceConfigurationError|LinkageError guard alongside next().
- ITXTableSparkRuntimeBundle: drain spark-submit stdout on a background thread
  so a hung process is caught by waitFor(10min) instead of blocking forever on
  readOutput() reading until EOF.

* [836] ci: point Maven CI + License Check triggers at branch-0.4

main-hudi-0x was deleted and replaced by branch-0.4 (the 0.4.0 release line),
so update the push/pull_request branch filters accordingly.

* [836] ci: validate xtable-spark-runtime bundle against a real Spark distro

ITXTableSparkRuntimeBundle spark-submits the shaded bundle jar and
self-skips unless SPARK_HOME is set, so the main CI never runs it. Add a
path-filtered workflow that installs a matching Spark 3.4 distribution
from the Apache archive, sets SPARK_HOME, and runs the failsafe IT.

* [836] ci: run spark-runtime validation on every branch PR (drop path filter)

The workflow is intended to be a required status check on branch-0.4. A
required check whose workflow is skipped by a path filter never reports,
leaving PRs blocked on a pending check, so run it unconditionally on the
covered branches. The Spark distro is cached, so the added cost is the
reactor build plus the ~20s IT.

* [836] xtable-spark-runtime: address review feedback (CLI validation, unix flags, docs)

- XTableSparkSync: validate --sourceformat/--targets up front and fail fast
  (before SparkSession creation) on empty or unsupported values. Split the
  allowed sets: sources are Hudi/Iceberg/Delta/Paimon/Parquet, targets are
  Hudi/Iceberg/Delta (Paimon/Parquet are read-only, no ConversionTarget).
- XTableSyncService.sourceProviderFor: wire Paimon and Parquet source providers
  (previously threw UnsupportedOperationException); engines remain cluster-provided.
- Rename CLI long-opts to unix-style lowercase (--basepath, --sourceformat,
  --tablename, --datapath, --partitionspec); update javadoc, IT and RFC example.
- basePathToName -> basePathToTableName: handle "/", trailing slashes and null
  by throwing with a "pass --tablename" hint instead of an empty table name.
- Add XTableSparkSyncTest covering table-name derivation and source/target
  format validation.
- ConversionTargetFactory: log the skipped provider's error class/message so
  operators can distinguish an intentionally-absent engine from a linkage error.
- spark-runtime-validation.yml: add a workflow_dispatch spark_version input and
  document why the Spark 3.4 line is pinned (Delta 2.4.0 is Spark-3.4-only).
- RFC-3: add a supported-formats/engine-versions section, "from application
  code" (Scala/Java + PySpark) activation examples, and list @vinothchandar as
  an approver.

* [836] xtable-spark-runtime: support Spark 3.5 via Delta Kernel

Delta source/target now auto-switch from delta-core to the Spark-free
Delta Kernel implementation on Spark 3.5+ (where the bundled delta-core
does not run), controlled by a single --usedeltakernel toggle that is
auto-enabled by Spark version. Hudi/Iceberg sync was already Spark-free.

Also runs the spark-runtime bundle IT on both Spark 3.4.3 and 3.5.9 via
a CI matrix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* [836] Give Maven CI Build and License Check unique job names

Both workflows used a job named "build", so both reported the same
status-check context and could not be required distinctly. Set unique
job names so branch protection (see #848) can
require each on branch-0.4.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* [836] xtable-spark-runtime: add --datasetconfig for multi-table sync

XTableSparkSync accepts a --datasetconfig YAML (sourceFormat, targetFormats,
datasets[]) to sync multiple tables in one run, mutually exclusive with the
single-table --basepath/--sourceformat/--targets flags. The config is read
through the Spark Hadoop config, so it may live on a local or cloud
(s3/gcs/abfs) path, and reuses the same schema as the RunSync utility.

Parsed with SnakeYAML's SafeConstructor (plain maps/lists only, no arbitrary
type instantiation). SnakeYAML is bundled and relocated to
org.apache.xtable.shaded (like guava/commons-cli) because Spark ships its own
version (1.33 on 3.4, 2.0 on 3.5) that would otherwise clash; its multi-release
classes are filtered out so no un-relocated org.yaml classes remain.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@rangareddy rangareddy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread .asf.yaml
Comment on lines +49 to +50
- "validate-spark-runtime-bundle (3.4.3)"
- "validate-spark-runtime-bundle (3.5.9)"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to use a wildcard so we don't need the versions listed here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants