comphead commented on code in PR #7516:
URL: https://github.com/apache/datafusion-comet/pull/7516#discussion_r4485578266
##########
dev/ci/check-ci-config.py:
##########
@@ -401,17 +388,16 @@ def fork_pr(action="synchronize", label=None, labels=(),
base="main", repository
},
{"spark_4_1_hive"},
),
- # Adding the hive label on top of the suite label re-runs only the hive
- # rows: a `labeled` run selects what the new label gates, and the suite
- # label's earlier run already covered every module at this commit.
+ # The retired suite label gates nothing now, so it must not start a second
+ # pipeline or add anything to a pushed commit's run.
Review Comment:
This comment says the retired label must not "add anything to a pushed
commit's run", but the case below only covers the `labeled` event. Could you
add a `synchronize` case with `labels: ["run-spark-4.1-tests"]` that expects
`PR_TIER`? Without it nothing in the checker would notice if the label started
gating `spark_4_1_hive` again. I evaluated `compute()` for that event on the
head and got only `PR_TIER`, so the case should pass as is.
##########
dev/ci/compute-changes.py:
##########
@@ -583,21 +583,23 @@
# their label.
"spark_3_5": ["nightly", "label:run-spark-3.5-tests"],
"spark_4_0": ["nightly", "label:run-spark-4.0-tests"],
- # No Spark SQL suite runs on a plain pull request. Spark 4.1 was the last
- # one in the PR tier, first whole (issue #5870 pulled the sql_hive shards
- # out) and then catalyst and sql_core alone. What changed is how often a
- # pull request is pushed: with agent-driven review and agent-driven
- # replies to review, a PR now goes through several more rounds before it
- # is queued, and each round paid for the whole 4.1 build. The queue still
- # runs every shard before anything lands; the two labels bring the run
- # forward. `run-spark-4.1-tests` selects the whole suite, so it appears on
- # both outputs; `run-spark-4.1-hive-tests` selects only the hive shards.
- "spark_4_1": ["queue", "label:run-spark-4.1-tests"],
- "spark_4_1_hive": [
- "queue",
- "label:run-spark-4.1-tests",
- "label:run-spark-4.1-hive-tests",
- ],
+ # Spark 4.1 is the default build profile, and its catalyst and sql_core
+ # shards are the one Spark SQL suite every pull request runs. For a pull
+ # request from a fork that is the fork's runners, not ASF's: the PR tier
+ # runs there (see UPSTREAM below). The suite was out of the PR tier while
+ # the PR tier ran upstream, because agent-driven review rounds meant each
+ # pull request paid for the whole 4.1 build several times over. On a fork
+ # that repetition costs ASF nothing, and running the suite there means a
+ # contributor sees a 4.1 failure before the queue does, rather than having
+ # to ask for a label. The queue runs the same shards again on the merge
+ # result, and it stays the gate.
+ #
+ # The sql_hive shards stay in the queue alone. They are four more shards
+ # on every push for a surface few changes touch, and the queue still runs
+ # them before anything lands; `run-spark-4.1-hive-tests` brings them
+ # forward onto a pull request.
+ "spark_4_1": ["pr", "queue"],
+ "spark_4_1_hive": ["queue", "label:run-spark-4.1-hive-tests"],
Review Comment:
There are 42 open pull requests that carry `run-spark-4.1-tests` (all from
forks). Until now the label set both `spark_4_1` and `spark_4_1_hive`, so their
upstream runs included the `sql_hive` shards. After this change a push to any
of them runs only catalyst and `sql_core`, and the hive shards wait for the
queue, with no message saying so. Should the PR description tell those authors
to apply `run-spark-4.1-hive-tests` if they want the hive shards early? Or
would keeping `label:run-spark-4.1-tests` on `spark_4_1_hive` until the label
is deleted be less surprising?
--
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]
