philo-he commented on code in PR #12745: URL: https://github.com/apache/gluten/pull/12745#discussion_r3763861756
########## docs/developers/velox-backend-CI.md: ########## @@ -38,4 +38,11 @@ Since the Docker image is rebuilt weekly, the ccache is mostly outdated, so it i ## Updating the Docker Image The GitHub secrets `DOCKERHUB_USER` and `DOCKERHUB_TOKEN` are used to push Docker images to [Docker Hub](https://hub.docker.com/r/apache/gluten/tags). -Note that GitHub secrets are not accessible in PRs from forked repos. \ No newline at end of file +Note that GitHub secrets are not accessible in PRs from forked repos. + +## Delta Spark UT +`delta_spark_ut.yml` runs delta-io/delta's own `spark` test suite against a Gluten Velox bundle, so Gluten is validated against a real Delta release. A number of those tests fail today. Not because Gluten declines to offload a plan -- that should fall back to vanilla Spark and the test should still pass. Some are real gaps (fallback not happening where it should, metrics that differ from vanilla, native-side bugs), and some are expected: a test that asserts on the query plan sees a different plan once the scan or operators are offloaded, which is by design rather than a defect. So the job does not gate on "any failure": it compares each run against a committed baseline of known failures in `.github/workflows/util/delta-spark-ut/known-failures.txt` and fails on a **new** failure, or on a baseline test that starts **passing** (which means the baseline needs updating). It also fails outright if a run produced no usable results -- missing or truncated JUnit reports, or fewer shards than expected -- rather than passing on partial data. Review Comment: Nit: All of this text sits on line 44, which is hard to work with in an editor. It also means a future one-word edit will show the entire line as changed in the diff. Generally it's better to wrap at 100-150 characters, or put each sentence on its own line. Same applies to the other long lines in this document. Thanks! ########## .github/workflows/delta_spark_ut.yml: ########## @@ -57,6 +57,13 @@ on: - 'gluten-delta/**' # Covers src-delta, src-delta33, src-delta40 and any future variant. - 'backends-velox/src-delta*/**' + # Documentation can't change what the suite does, and this job costs ~10 + # runner-hours, so don't spend that on a README edit. `**/*.md` is the + # documented spelling for "a .md file anywhere under this directory" and + # covers both the README and any nested notes. Negated patterns are Review Comment: Nit: Can we remove "Documentation can't chang ...and any nested notes." for brevity? -- 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]
