-
Notifications
You must be signed in to change notification settings - Fork 46
[SPARK-49272] Update Dockerfile to use alpine/java:17-jdk instead of gradle:8.9.0-jdk17-jammy
#54
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
Conversation
… of `gradle:8.9.0-jdk17-jammy`
3d6c0fe to
a2c2cf1
Compare
| fi | ||
| # If the file still doesn't exist, let's try `wget` and cross our fingers | ||
| if [ ! -e $APP_HOME/gradle/wrapper/gradle-wrapper.jar -a "$(command -v wget)" ]; then | ||
| wget -O $APP_HOME/gradle/wrapper/gradle-wrapper.jar https://raw.githubusercontent.com/gradle/gradle/v8.10.0/gradle/wrapper/gradle-wrapper.jar |
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.
In GitHub Action environment, DNS setup could fail like the following. So, we need to try to fallback to IP in the worst case.
#14 [builder 4/4] RUN ./gradlew clean build -x check
#14 5.254 wget: bad address 'raw.githubusercontent.com'
|
Could you review this, @viirya ? |
|
Looks good to me. Thanks @dongjoon-hyun. |
|
Thank you, @viirya ! |
|
Oh, after merging, it seems that the commit builder fails. |
|
Let me revert this and investigate it more. |
|
Got it. Please ping me on your new PR once it is ready. |
|
I tried to use different images like |
…che#54) ## What changes were proposed in this pull request? This updates the rules for operator role & disables probe by default ## Why are the changes needed? AppleCluster is an interim CR that is needed during cluster API migration. Probes are keep failing in both Kube and AIML, this disable the probes for deployment while the root cause being investigated ## Does this PR introduce any user-facing change? No ## How was this patch tested? Helm lint ## Was this patch authored or co-authored using generative AI tooling? No
…tead of `ToStringBuilder` ### What changes were proposed in this pull request? This PR aims to improve `SentinelResourceState.toString` by JEP-280 instead of `ToStringBuilder`. ### Why are the changes needed? This is aligned with Apache Spark main repository improvement. - apache/spark#51572 Since Java 9, `String Concatenation` has been handled better by default. | ID | DESCRIPTION | | - | - | | JEP-280 | [Indify String Concatenation](https://openjdk.org/jeps/280) | For example, `SentinelResourceState.toString` is changed like the following by this PR. **BEFORE** ``` public java.lang.String toString(); Code: 0: new #43 // class org/apache/commons/lang3/builder/ToStringBuilder 3: dup 4: aload_0 5: invokespecial #45 // Method org/apache/commons/lang3/builder/ToStringBuilder."<init>":(Ljava/lang/Object;)V 8: ldc #48 // String resource 10: aload_0 11: getfield #17 // Field resource:Lorg/apache/spark/k8s/operator/BaseResource; 14: invokevirtual #49 // Method org/apache/commons/lang3/builder/ToStringBuilder.append:(Ljava/lang/String;Ljava/lang/Object;)Lorg/apache/commons/lang3/builder/ToStringBuilder; 17: ldc #53 // String previousGeneration 19: aload_0 20: getfield #39 // Field previousGeneration:J 23: invokevirtual #54 // Method org/apache/commons/lang3/builder/ToStringBuilder.append:(Ljava/lang/String;J)Lorg/apache/commons/lang3/builder/ToStringBuilder; 26: ldc #57 // String isHealthy 28: aload_0 29: getfield #13 // Field isHealthy:Z 32: invokevirtual #58 // Method org/apache/commons/lang3/builder/ToStringBuilder.append:(Ljava/lang/String;Z)Lorg/apache/commons/lang3/builder/ToStringBuilder; 35: invokevirtual #61 // Method org/apache/commons/lang3/builder/ToStringBuilder.toString:()Ljava/lang/String; 38: areturn ``` **AFTER** ``` public java.lang.String toString(); Code: 0: aload_0 1: getfield #17 // Field resource:Lorg/apache/spark/k8s/operator/BaseResource; 4: invokestatic #43 // Method java/lang/String.valueOf:(Ljava/lang/Object;)Ljava/lang/String; 7: aload_0 8: getfield #39 // Field previousGeneration:J 11: aload_0 12: getfield #13 // Field isHealthy:Z 15: invokedynamic #49, 0 // InvokeDynamic #0:makeConcatWithConstants:(Ljava/lang/String;JZ)Ljava/lang/String; 20: areturn ``` ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #369 from dongjoon-hyun/SPARK-53818. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
What changes were proposed in this pull request?
This PR aims the following.
alpine/java:17-jdkinstead ofgradle:8.9.0-jdk17-jammyas a Docker image builder image.gradesby adding a last fallback to handle DNS error like the following.Why are the changes needed?
alpineJava 17 image is 65% smaller. And, we currently use grade wrapper to download newer gradle zip file.Technically, we already upgraded to
Gradle 8.10.0and we don't use8.9.0.In addition,
185.199.111.133is one of the official IP ofraw.githubusercontent.com.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Pass the CIs.
Was this patch authored or co-authored using generative AI tooling?
No.