Code review comment for lp:~hduran-8/juju-core/apiworker_force_local_connection

On 2014/05/30 14:12:32, wwitzel3 wrote:

https://codereview.appspot.com/100810045/diff/40001/state/api/apiclient.go
> File state/api/apiclient.go (right):

https://codereview.appspot.com/100810045/diff/40001/state/api/apiclient.go#newcode111
> state/api/apiclient.go:111:
> We should use the built-in sort here.

> http://golang.org/pkg/sort/

> type LocalhostFirst []string

> func (a LocalhostFirst) Len() int { return len(a) }
> func (a LocalhostFirst) Swap(i int, j int) { a[i], a[j] = a[j], a[i] }
> func (a LocalhostFirst) Less(i int, j int) bool { return
strings.HasPrefix(a[i],
> "localhost") }

> return sort.Sort(LocalhostFirst(addrs))

So I think the sort.Sort is actually incorrect, having worked in this
code a bit.

Specifically, this breaks the order for *everything else*. And we
intentionally move the last-successful-connection to the beginning of
the list, so the order isn't random.

So for things like EC2, where you have public and private (etc)
addresses, this now essentially randomizes which one we are going to try
to connect to first. (Actually, it probably sorts the private addresses
first because 10. comes before 172.).

I can see the point of wanting to try localhost first, and in HA mode we
really do want localhost. However, I think what we *really* want is to
check "if JobManageEnviron" then *only* connect to localhost, right?

https://codereview.appspot.com/100810045/