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

feat: update graal-sdk to 24 and graalvm-A to 21.x #815

Merged
merged 22 commits into from
Jul 30, 2024
Merged

Conversation

mpeddada1
Copy link
Contributor

@mpeddada1 mpeddada1 commented Apr 24, 2024

Also verified locally with java-kms (logs).

Noting two significant changes in PR:

@product-auto-label product-auto-label bot added the size: xs Pull request size is extra small. label Apr 24, 2024
@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: xs Pull request size is extra small. labels Apr 25, 2024
@mpeddada1
Copy link
Contributor Author

mpeddada1 commented Apr 25, 2024

In JDK 21, ITHttpAnnotation fails with:

 [ERROR] Tests run: 4, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.460 s <<< FAILURE! -- in com.google.showcase.v1beta1.it.ITHttpAnnotation
Step #1: [ERROR] com.google.showcase.v1beta1.it.ITHttpAnnotation.testComplianceGroup[Compliance Group Name: Fully working conversions, no resources] -- Time elapsed: 0.031 s <<< ERROR!
Step #1: com.google.api.gax.rpc.InvalidArgumentException: Bad Request
Step #1: 	at com.google.api.gax.rpc.ApiExceptionFactory.createException(ApiExceptionFactory.java:52)
Step #1: 	at com.google.api.gax.httpjson.HttpJsonApiExceptionFactory.createApiException(HttpJsonApiExceptionFactory.java:76)
Step #1: 	at com.google.api.gax.httpjson.HttpJsonApiExceptionFactory.create(HttpJsonApiExceptionFactory.java:54)
Step #1: 	at com.google.api.gax.httpjson.HttpJsonExceptionCallable$ExceptionTransformingFuture.onFailure(HttpJsonExceptionCallable.java:97)
. . .
Step #1: Caused by: com.google.api.client.http.HttpResponseException: 400 Bad Request
Step #1: POST http://localhost:7469/v1beta1/repeat:body
Step #1: {"error":{"code":400,"message":"error reading body params '*': proto: syntax error (line 1:513): invalid value \u0000","details":null,"Body":"","Header":null,"Errors":null}}
Step #1: 	at com.google.api.client.http.HttpResponseException$Builder.build(HttpResponseException.java:293)
Step #1: 	at com.google.api.client.http.HttpRequest.execute(HttpRequest.java:1118)
Step #1: 	at com.google.api.gax.httpjson.HttpRequestRunnable.run(HttpRequestRunnable.java:115)
Step #1: 	... 6 more

However, the same test passes in JDK 11.

@mpeddada1
Copy link
Contributor Author

Running the same test multiple times locally on JDK 21 shows this test to failing 15/20 times

Screenshot 2024-04-25 at 6 29 20 PM

@mpeddada1
Copy link
Contributor Author

Created issue to track ITHttpAnnotation failure in googleapis/sdk-platform-java#2695

@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 Apr 30, 2024
@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: m Pull request size is medium. labels Apr 30, 2024
@mpeddada1 mpeddada1 changed the title feat: update graal-sdk to 24 and graalvm-A to 22.x feat: update graal-sdk to 24 and graalvm-A to 21.x Apr 30, 2024
@mpeddada1
Copy link
Contributor Author

mpeddada1 commented Apr 30, 2024

downstream / dependencies (11, java-spanner) is resulting in:

[INFO] --- maven-dependency-plugin:3.5.0:analyze (default-cli) @ google-cloud-spanner ---
Error:  Used undeclared dependencies found:
Error:     org.graalvm.sdk:nativeimage:jar:24.0.1:provided

According to the graal/sdk CHANGELOG, the GraalVM SDK was split into smaller modules and the use of graal-sdk is deprecated. The new maven configuration to customize native image generation is:

<dependency>
  <groupId>org.graalvm.sdk</groupId>
  <artifactId>nativeimage</artifactId>
  <version>${graalvm.version}</version>
</dependency>

java-spanner's pom needs to be updated to ignore the provided org.graalvm.sdk:nativeimage dependency: https://github.com/googleapis/java-spanner/blob/206534cb410c2ba2f4099a4fb5fb414e05ba07a4/google-cloud-spanner/pom.xml#L153

@burkedavison burkedavison added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 9, 2024
@burkedavison
Copy link
Member

Do not merge until googleapis/sdk-platform-java#2695 is resolved

@googleapis googleapis deleted a comment from Joanna2473 Jul 29, 2024
@googleapis googleapis deleted a comment from Joanna2473 Jul 29, 2024
@googleapis googleapis deleted a comment from Joanna2473 Jul 29, 2024
@burkedavison
Copy link
Member

@mpeddada1 : Do you agree this can now be merged?

@mpeddada1 mpeddada1 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 30, 2024
@mpeddada1
Copy link
Contributor Author

mpeddada1 commented Jul 30, 2024

Following up on #815 (comment), looked into adding the maven-dependency-plugin settings to native-image-shared-config in #815 (comment) but these settings end up getting overridden by the child pom so it doesn't address the issue we are facing with java-spanner.

Current approach: Add ignoreDependencies property in native-image-shared-config to be included downstream when this artifact is released and updated in java-spanner. The side effect of this is that the java-spanner repo will temporarily see a failure.

@mpeddada1 mpeddada1 merged commit 3c3c630 into main Jul 30, 2024
59 of 60 checks passed
@mpeddada1 mpeddada1 deleted the udpate-graalvm-a branch July 30, 2024 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants