Repository navigation
chore(dev/release): fix verification issues #4937
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
| # under the License. | ||
|
|
||
| cython | ||
| libpython | ||
| # nodejs is required by pyright | ||
| nodejs >=13.0.0 | ||
| pandas | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -838,6 +838,7 @@ test_unix_wheels() { | |
| adbc_*-cp${pyver/.}-cp${python/.}-${platform}*${arch}*.whl \ | ||
| adbc_*-py3-none-${platform}*${arch}*.whl | ||
| ${ADBC_DIR}/ci/scripts/python_wheel_unix_test.sh ${ADBC_DIR} | ||
| unset PYTHON_GIL | ||
| done | ||
| } | ||
|
|
||
|
|
@@ -887,7 +888,7 @@ test_wheels() { | |
|
|
||
| test_jars() { | ||
| show_header "Testing Java jars" | ||
| maybe_setup_conda maven python || exit 1 | ||
| maybe_setup_conda maven openjdk=17 python || exit 1 | ||
|
|
||
| local root_poms=() | ||
| shopt -s nullglob | ||
|
|
@@ -913,11 +914,25 @@ print(version.text) | |
| show_info "ADBC Java version: ${java_version}" | ||
|
|
||
| local -r maven_repository="${ARROW_TMPDIR}/maven-repository" | ||
| local -r maven_repository_argument="-Dmaven.repo.local=${maven_repository}" | ||
| local -r maven_arguments=( | ||
| -B | ||
| -e | ||
| # Recheck missing artifacts instead of reusing cached resolution failures. | ||
| -U | ||
| "-Dmaven.repo.local=${maven_repository}" | ||
| # Retry transient download failures in Maven's native HTTP transport. | ||
| -Dmaven.resolver.transport=native | ||
| -Daether.connector.http.retryHandler.count=5 | ||
| -Daether.connector.http.retryHandler.interval=5000 | ||
| -Daether.connector.http.retryHandler.serviceUnavailable=429,500,502,503,504 | ||
| # Maven 3.10+ uses Resolver 2, which renamed the HTTP transport properties. | ||
| -Daether.transport.http.retryHandler.count=5 | ||
| -Daether.transport.http.retryHandler.interval=5000 | ||
| -Daether.transport.http.retryHandler.serviceUnavailable=429,500,502,503,504 | ||
| ) | ||
| mkdir -p "${maven_repository}" | ||
|
|
||
| mvn -B install:install-file \ | ||
| "${maven_repository_argument}" \ | ||
| mvn "${maven_arguments[@]}" install:install-file \ | ||
| "-Dfile=${root_pom}" \ | ||
| "-DpomFile=${root_pom}" \ | ||
| -Dpackaging=pom | ||
|
|
@@ -944,16 +959,21 @@ print(version.text) | |
| fi | ||
| done | ||
|
|
||
| mvn -B install:install-file \ | ||
| "${maven_repository_argument}" \ | ||
| mvn "${maven_arguments[@]}" install:install-file \ | ||
| "-Dfile=${jar_path}" \ | ||
| "-DpomFile=${pom_path}" \ | ||
| "-Dsources=${sources_path}" \ | ||
| "-Djavadoc=${javadoc_path}" | ||
| done | ||
|
|
||
| mvn -B test \ | ||
| "${maven_repository_argument}" \ | ||
| local test_arguments=() | ||
| if [[ "$(uname)" == "Darwin" && "$(uname -m)" == "x86_64" ]]; then | ||
| show_info "Skipping testDriverJni: JNI binaries are not available for macOS/x86_64" | ||
| test_arguments+=('-Dtest=!VerifyReleaseCandidateTest#testDriverJni') | ||
| fi | ||
|
|
||
| mvn "${maven_arguments[@]}" test \ | ||
| "${test_arguments[@]}" \ | ||
|
Comment on lines
+969
to
+976
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this be cleaner if we put this in the Java test? diff --git a/dev/release/verify-release-candidate.sh b/dev/release/verify-release-candidate.sh
index 3c8e123f2..c59fa0fdc 100755
--- a/dev/release/verify-release-candidate.sh
+++ b/dev/release/verify-release-candidate.sh
@@ -966,14 +966,7 @@ print(version.text)
"-Djavadoc=${javadoc_path}"
done
- local test_arguments=()
- if [[ "$(uname)" == "Darwin" && "$(uname -m)" == "x86_64" ]]; then
- show_info "Skipping testDriverJni: JNI binaries are not available for macOS/x86_64"
- test_arguments+=('-Dtest=!VerifyReleaseCandidateTest#testDriverJni')
- fi
-
mvn "${maven_arguments[@]}" test \
- "${test_arguments[@]}" \
"-Dadbc.version=${java_version}" \
-f "${SOURCE_DIR}/verify/java/pom.xml"
}
diff --git a/dev/release/verify/java/src/test/java/org/apache/arrow/adbc/verify/VerifyReleaseCandidateTest.java b/dev/release/verify/java/src/test/java/org/apache/arrow/adbc/verify/VerifyReleaseCandidateTest.java
index 83f6d5512..03c4abfcc 100644
--- a/dev/release/verify/java/src/test/java/org/apache/arrow/adbc/verify/VerifyReleaseCandidateTest.java
+++ b/dev/release/verify/java/src/test/java/org/apache/arrow/adbc/verify/VerifyReleaseCandidateTest.java
@@ -85,6 +85,17 @@ class VerifyReleaseCandidateTest {
@Test
void testDriverJni() {
var driver = new JniDriver(allocator);
+ if (OS.MAC.isCurrentOs() && "x86_64".equalsIgnoreCase(System.getProperty("os.arch"))) {
+ // JNI binaries are not available for macOS x86_64.
+ ExceptionInInitializerError exception =
+ assertThrows(
+ ExceptionInInitializerError.class, () -> driver.load().driver("nonexistent").open());
+ assertThat(exception)
+ .hasRootCauseInstanceOf(RuntimeException.class)
+ .hasRootCauseMessage("ADBC JNI driver not supported on architecture x86_64");
+ return;
+ }
+
if (OS.WINDOWS.isCurrentOs() && "aarch64".equalsIgnoreCase(System.getProperty("os.arch"))) {
// JNI binaries are not available for Windows ARM64.
ExceptionInInitializerError exception =
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would, but then I'd have to cut the release from scratch
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Want to file an issue and we can get this fixed for the next release?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, of course
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| "-Dadbc.version=${java_version}" \ | ||
| -f "${SOURCE_DIR}/verify/java/pom.xml" | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems out of place but safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, it turns out that the Conda environment activate/deactivate was barfing if it was set.