Support ChooseHostInterface on Mac OS X #5497
Conversation
| inFile, err := os.Open("/proc/net/route") | ||
| if err != nil { | ||
| if os.IsNotExist(err) { | ||
| return chooseHostInterfaceNativeGo() | ||
| return chooseHostInterfaceNativeGo(nw) |
There was a problem hiding this comment.
log this happening - it's important in prod
|
I think that we want unit tests to explicitly use 127.0.0.1 (as per your PR 5400) and for ChooseHostInterface to do precisely the correct thing on a given platform (e.g. based on /proc/net/route on linux, perhaps Ultimately we need to implement a different version of ChooseHostInterface() for each supported platform, correct in each case, using arch-specific compilation. You could either do this now while it's fresh in your mind, or defer until later, as it doesn't appear that we need the functionality yet, given that the specific testing problem is resolved by PR 5400. |
|
PS: To be clear, the overall goal of the above suggestion is to make sure that unreliable code like ChooseHostInterfaceNativeGo() does not accidentally leak from testing code into production code. |
|
I think I agree - if I had been paying attention I would have argued I think that any test which calls ChooseHostInterface() is already doomed. I think that ChooseHostinterface is a linux-specific thing that we should On Wed, Mar 18, 2015 at 3:20 PM, Quinton Hoole notifications@github.com
|
|
Closing this as it is unmergeable and hasn't been worked on in > 30 days. Please re-open with fresh changes if the PR is still relevant. |
|
Ack - I'll open a separate PR to remove ChooseHostInterfaceNativeGo() |
ghost
mentioned this pull request
|
ChooseHostInterface can choose an IPv6 only address in some cases, which we don't handle. I'm going to spawn a separate issue, but we're still busted in some cases. |
|
Spawned #7716 |
|
@smarterclayton @quinton-hoole Since #7721 has merged Do we still need to do something here? |
|
No. |
@thockin fixed the public method issue , @brendanburns also support for mac where route file is not present.
kindly review