Skip to content
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

Merged
merged 25 commits into from May 25, 2022

Conversation

mpeddada1
Copy link
Contributor

Fixes #2007

@mpeddada1 mpeddada1 requested a review from a team as a code owner May 18, 2022 20:17
@mpeddada1 mpeddada1 requested review from a team and GaoleMeng May 18, 2022 20:17
@product-auto-label product-auto-label bot added size: s Pull request size is small. api: bigquery Issues related to the googleapis/java-bigquery API. labels May 18, 2022
@mpeddada1 mpeddada1 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 18, 2022
@mpeddada1 mpeddada1 changed the title fix: add more native image configurations for Arrow tests fix: [wip] add more native image configurations for Arrow tests May 18, 2022
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels May 19, 2022
@mpeddada1 mpeddada1 changed the title fix: [wip] add more native image configurations for Arrow tests fix: add more native image configurations for Arrow tests May 19, 2022
@mpeddada1 mpeddada1 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 19, 2022
@mpeddada1
Copy link
Contributor Author

Local testing succeeded so removing the "do not merge" label.

@generated-files-bot
Copy link

Warning: This pull request is touching the following templated files:

  • .kokoro/build.sh

@mpeddada1 mpeddada1 changed the title fix: add more native image configurations for Arrow tests fix: add more native image configurations for Arrow tests and enable native image tests May 19, 2022
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 20, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 20, 2022
.kokoro/build.sh Show resolved Hide resolved
@@ -3,8 +3,67 @@
"name":"io.netty.buffer.AbstractByteBufAllocator",
"queryAllDeclaredMethods":true
},
{
"name":"io.netty.buffer.PooledByteBufAllocator",
Copy link
Contributor

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

Copy link
Contributor Author

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.

@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 23, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 23, 2022
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 23, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 23, 2022
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 23, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 23, 2022
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 24, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 24, 2022
@@ -22,5 +22,5 @@
'.kokoro/dependencies.sh',
'codecov.yaml',
'renovate.json',
'.kokoro/build.sh',
'.kokoro/build.sh'
Copy link
Contributor

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?

Copy link
Contributor Author

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.

@mpeddada1
Copy link
Contributor Author

All checks including Kokoro JDK 17 native image are now passing.

@stephaniewang526 stephaniewang526 merged commit 7f0bfd4 into main May 25, 2022
@stephaniewang526 stephaniewang526 deleted the fix-arrow-tests branch May 25, 2022 15:15
gcf-merge-on-green bot pushed a commit that referenced this pull request May 25, 2022
🤖 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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the googleapis/java-bigquery API. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate GraalVM IT failure with Apache Arrow
3 participants