[SPARK-28886][K8S] Fix the DepsTestsSuite with minikube 1.3.1 #25599
Conversation
…xt when printing service URL
|
Kubernetes integration test starting |
|
Test build #109833 has finished for PR 25599 at commit
|
|
Kubernetes integration test status success |
| url | ||
| case _ => | ||
| logWarning(s"Response from minikube ${rawUrl} did not match URL regex") | ||
| rawUrl |
There was a problem hiding this comment.
One question -- did this ever work before? meaning, would we ever get a response that doesn't match the regex legitimately? If so should it be a warning? Or is it that this doesn't work at all right now?
There was a problem hiding this comment.
So I've only tested on OSX and just incase someone elses setup doesn't match the regex (which I'm not sure would work) I figured a fall-through with a warning was a good approach. I also considered just throwing here.
There was a problem hiding this comment.
Out of curiosity what does the minikube command return for you? wondering if it's something that varies by version or we have a problem with the command outputting extra stuff. Seems like it intends to only return a URL.
There was a problem hiding this comment.
Talking with Stavros, this was not happening on previous version of minikube, so if we strip it, we just need to be careful it also works with earlier revs.
There was a problem hiding this comment.
If the problem is that it's * some://url:port[whitespace] then this looks like a fine fix. Would it help unblock other PRs?
There was a problem hiding this comment.
It should be backward compatible. No junk prefix, then that subexpression is empty.
There was a problem hiding this comment.
My bad for not responding sooner, yeah it's fine :)
dongjoon-hyun
changed the title
|
Merged to master |
### What changes were proposed in this pull request? Matches the response from minikube service against a regex to extract the URL ### Why are the changes needed? minikube 1.3.1 on OSX has different formatting than expected ### Does this PR introduce any user-facing change? No ### How was this patch tested? Ran the existing integration test run on OSX with minikube 1.3.1 Closes apache#25599 from holdenk/SPARK-28886-fix-deps-tests-with-minikube-1.3.1. Authored-by: Holden Karau <hkarau@apple.com> Signed-off-by: Sean Owen <sean.owen@databricks.com>
5 participants
What changes were proposed in this pull request?
Matches the response from minikube service against a regex to extract the URL
Why are the changes needed?
minikube 1.3.1 on OSX has different formatting than expected
Does this PR introduce any user-facing change?
No
How was this patch tested?
Ran the existing integration test run on OSX with minikube 1.3.1