jbonofre commented on code in PR #1430:
URL: https://github.com/apache/arrow-java/pull/1430#discussion_r4443331341
##########
.github/workflows/rc.yml:
##########
@@ -267,6 +271,14 @@ jobs:
# make brew Java available to CMake
export JAVA_HOME=$(brew --prefix
openjdk@17)/libexec/openjdk.jdk/Contents/Home
ci/scripts/jni_macos_build.sh . arrow build jni
+ - name: Test JSON dataset read
Review Comment:
This step assumes the macOS native build includes JSON support, but
`jni_macos_build.sh` exports `ARROW_DATASET`, `ARROW_PARQUET`, `ARROW_ORC` and
`ARROW_GANDIVA`, and not `ARROW_JSON`.
If the preset or the released C++ doesn't enable it, `testBaseJsonRead`
fails with `FileFormat.JSON` only after the long native build.
We should double check that JSON is enabled and export `ARROW_JSON=ON` in
the build script.
Also, `-Dsurefire.failIfNoSpecifiedTests=false` means this step passes
silently if the test is renamed or removed. Dropping that flag would make the
step fail in that case.
##########
.github/workflows/rc.yml:
##########
@@ -178,7 +178,11 @@ jobs:
- name: Extract source archive
run: |
tar -xf apache-arrow-java-*.tar.gz --strip-components=1
+ - name: Download the latest Apache Arrow C++
Review Comment:
This step downloads the latest released Arrow C++ tarball (`download_cpp.sh`
defaults to `latest-release`). That's only the case for push, PR and tag runs,
because the nightly schedule still builds against `apache/arrow` `main`.
The Linux JNI job above also still checks out `main`, so the two JNI jobs
now compile against different C++ versions.
I would suggest to use the same C++ source for both jobs, and pass
`latest-rc` explicitly, since this is the RC workflow.
As written, a JNI change that needs a newer C++ API could pass on one
trigger type and fail on another.
--
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]
