-
Notifications
You must be signed in to change notification settings - Fork 29.1k
[SPARK-30715][K8S][TESTS][FOLLOWUP] Update k8s client version in IT as well #27948
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
|
Kubernetes integration test starting |
|
Kubernetes integration test status success |
|
Test build #119990 has finished for PR 27948 at commit
|
| protected var appLocator: String = _ | ||
|
|
||
| // Default memory limit is 1024M + 384M (minimum overhead constant) | ||
| private val baseMemory = s"${1024 + 384}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.
Hi, @ScrapCodes . Do we need this change in this PR?
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.
Hi Dongjoon, Yes, it is part of updating the client version. Tests won't pass otherwise.
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.
To be precise, in 4.7.1 Quantity constructor tries to parse the amount into format and the amount. link
And in 4.6.4, It does not. link
More details here : fabric8io/kubernetes-client@c370d26
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.
Hi @dongjoon-hyun, a gentle ping.
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.
Thanks!
dongjoon-hyun
left a comment
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.
+1, LGTM. Merged to master.
Thank you, @ScrapCodes .
…s well ### What changes were proposed in this pull request? This is a follow up for SPARK-30715 . Kubernetes client version in sync in integration-tests and kubernetes/core ### Why are the changes needed? More than once, the kubernetes client version has gone out of sync between integration tests and kubernetes/core. So brought them up in sync again and added a comment to save us from future need of this additional followup. ### Does this PR introduce any user-facing change? No ### How was this patch tested? Manually. Closes apache#27948 from ScrapCodes/follow-up-spark-30715. Authored-by: Prashant Sharma <prashsh1@in.ibm.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
|
Hi. |
…s well ### What changes were proposed in this pull request? This is a follow up for SPARK-30715 . Kubernetes client version in sync in integration-tests and kubernetes/core ### Why are the changes needed? More than once, the kubernetes client version has gone out of sync between integration tests and kubernetes/core. So brought them up in sync again and added a comment to save us from future need of this additional followup. ### Does this PR introduce any user-facing change? No ### How was this patch tested? Manually. Closes #27948 from ScrapCodes/follow-up-spark-30715. Authored-by: Prashant Sharma <prashsh1@in.ibm.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org> (cherry picked from commit 3799d2b) Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
What changes were proposed in this pull request?
This is a follow up for SPARK-30715 . Kubernetes client version in sync in integration-tests and kubernetes/core
Why are the changes needed?
More than once, the kubernetes client version has gone out of sync between integration tests and kubernetes/core. So brought them up in sync again and added a comment to save us from future need of this additional followup.
Does this PR introduce any user-facing change?
No
How was this patch tested?
Manually.