-
Notifications
You must be signed in to change notification settings - Fork 121
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
fix: add more native image configurations for Arrow tests and enable native image tests #2053
Conversation
Local testing succeeded so removing the "do not merge" label. |
Warning: This pull request is touching the following templated files:
|
@@ -3,8 +3,67 @@ | |||
"name":"io.netty.buffer.AbstractByteBufAllocator", | |||
"queryAllDeclaredMethods":true | |||
}, | |||
{ | |||
"name":"io.netty.buffer.PooledByteBufAllocator", |
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.
Do you intend to manually maintain this config file in the future? How much ongoing maintenance work is needed? Would we need to make these changes if we made code changes related to Arrow or add new Arrow dependencies? cc @prash-mi
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.
That's a good question. Given that we have a bit more background on GraalVM at the moment, we plan to maintain these configurations with client library owners being involved in the review process. However, we're currently working on making our onboarding documentation more robust as we learn more about the technology so that it is easier for client library owners to contribute and get unblocked sooner if needed in the future . Will keep you posted on the plans for that.
Would we need to make these changes if we made code changes related to Arrow or add new Arrow dependencies?
Most of these changes involved configurations for the netty classes that Apache Arrow brings in. These currently exist in gax-grpc
but have been added here explicitly to avoid adding a dependency on gax-grpc. Since bigquery and most libraries bring in gax, we might want to consider moving them to gax
or even closer to netty which could help make this file a little shorter. As far as the Arrow dependencies are concerned, I don't have a solid answer for this but there is a possibility that we'll need to add new configurations if new deps are added. Luckily, we have our Kokoro GraalVM presubmit checks to catch them if required.
… into fix-arrow-tests
@@ -22,5 +22,5 @@ | |||
'.kokoro/dependencies.sh', | |||
'codecov.yaml', | |||
'renovate.json', | |||
'.kokoro/build.sh', | |||
'.kokoro/build.sh' |
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 should be removed, right?
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.
I had to bring this back in to exclude the arrow tests in Java 17 until #2060 is resolved.
All checks including Kokoro JDK 17 native image are now passing. |
🤖 I have created a release *beep* *boop* --- ## [2.12.0](v2.11.2...v2.12.0) (2022-05-25) ### Features * add build scripts for native image testing in Java 17 ([#1440](#1440)) ([#2057](#2057)) ([065ae78](065ae78)) ### Bug Fixes * add more native image configurations for Arrow tests and enable native image tests ([#2053](#2053)) ([7f0bfd4](7f0bfd4)) * Flaky testPositionalQueryParameters ([#2059](#2059)) ([3764b59](3764b59)) ### Dependencies * update dependency com.google.cloud:google-cloud-bigtable to v2.7.0 ([#2061](#2061)) ([1c7a0ab](1c7a0ab)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v2.11.0 ([#2055](#2055)) ([9667663](9667663)) * update dependency com.google.cloud:google-cloud-shared-dependencies to v2.12.0 ([#2063](#2063)) ([6d3f4be](6d3f4be)) * update dependency com.google.cloud:google-cloud-storage to v2.7.0 ([#2064](#2064)) ([fd47710](fd47710)) * update dependency com.google.cloud:google-cloud-storage to v2.7.1 ([#2066](#2066)) ([89962a5](89962a5)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Fixes #2007