Skip to content

Conversation

mpeddada1
Copy link
Contributor

@mpeddada1 mpeddada1 commented Mar 18, 2022

Relocating Spanner sample from GoogleCloudPlatform/native-image-support-java.
This PR also adds an integration test that can be run as a native image.

Calling mvn package -Pnative -DskipTests builds the native image for the application and calling mvn test -Pnative runs the test as a native image.

For more information: https://graalvm.github.io/native-build-tools/latest/maven-plugin.html#configuration.

cc @ansh0l @meltsufin

@mpeddada1 mpeddada1 requested review from a team as code owners March 18, 2022 20:35
@generated-files-bot
Copy link

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

  • samples/pom.xml
@product-auto-label product-auto-label bot added api: spanner Issues related to the googleapis/java-spanner API. samples Issues that are directly related to samples. labels Mar 18, 2022
@mpeddada1 mpeddada1 added the kokoro:run Add this label to force Kokoro to re-run the tests. label Mar 22, 2022
Copy link
Contributor

@ansh0l ansh0l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Minor nit: I think since the parent folder is named samples, we should call this one as native-image instead of native-image-sample

@ansh0l ansh0l added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Mar 29, 2022
@product-auto-label product-auto-label bot added the size: l Pull request size is large. label Apr 26, 2022
@mpeddada1
Copy link
Contributor Author

Thank you @ansh0l! I've renamed this to native-image

@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Apr 26, 2022
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 26, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 26, 2022
@ansh0l
Copy link
Contributor

ansh0l commented May 23, 2022

Closing since native image changes have been released with #1878

@ansh0l ansh0l closed this May 23, 2022
@mpeddada1 mpeddada1 reopened this May 25, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 1, 2022
@mpeddada1 mpeddada1 added kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Jun 1, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 1, 2022
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 1, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 1, 2022
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 1, 2022
@mpeddada1
Copy link
Contributor Author

mpeddada1 commented Jun 1, 2022

Rerunning because com.google.cloud.spanner.it.ITInstanceAdminTest is being flaky.

@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 1, 2022
@mpeddada1
Copy link
Contributor Author

All checks are now green.

Copy link
Contributor

@ansh0l ansh0l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a quick glance, looks alright. Please take a full review with @rajatbhatta

<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>libraries-bom</artifactId>
<version>25.2.0</version>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QQ> AFAIK, currently, we get automated PRs for cloud bom version bumps. Would now there be 2 such PRs (one for regular samples/pom.xml, another for samples/native-image/pom.xml), or would both end up coming into 1 PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would all be updated in a single PR. Here is an example: googleapis/java-pubsub#1132

@mpeddada1 mpeddada1 requested a review from rajatbhatta June 2, 2022 11:08
Co-authored-by: Rajat Bhatta <93644539+rajatbhatta@users.noreply.github.com>
Copy link
Contributor

@rajatbhatta rajatbhatta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 2, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 2, 2022
@mpeddada1 mpeddada1 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 2, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jun 2, 2022
@mpeddada1 mpeddada1 dismissed ansh0l’s stale review June 2, 2022 14:44

The requested changes have been addressed so modifying this to a request comment after receiving an approval from @rajatbhatta

@mpeddada1
Copy link
Contributor Author

Thank you both for the review!

@mpeddada1 mpeddada1 merged commit ef187f4 into main Jun 2, 2022
@mpeddada1 mpeddada1 deleted the move-native-sample branch June 2, 2022 14:44
gcf-merge-on-green bot pushed a commit that referenced this pull request Jun 22, 2022
🤖 I have created a release *beep* *boop* --- ## [6.25.6](v6.25.5...v6.25.6) (2022-06-22) ### Bug Fixes * PostgreSQL parser should not treat \ as an escape char ([#1921](#1921)) ([260bbe3](260bbe3)), closes [#1920](#1920) ### Documentation * **sample:** relocate native image sample from old repo ([#1758](#1758)) ([ef187f4](ef187f4)) ### Dependencies * update dependency org.graalvm.buildtools:native-maven-plugin to v0.9.11 ([#1907](#1907)) ([01f8a07](01f8a07)) * update dependency org.graalvm.buildtools:native-maven-plugin to v0.9.12 ([#1918](#1918)) ([be8b50b](be8b50b)) --- 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: spanner Issues related to the googleapis/java-spanner API. samples Issues that are directly related to samples. size: l Pull request size is large.

5 participants