Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2246)

Issue 103820044: Use localhost only if pressent for opening state

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 7 months ago by hduran
Modified:
5 years, 7 months ago
Reviewers:
mp+221788, jameinel, thumper
Visibility:
Public.

Description

Use localhost only if pressent for opening state Recently a change was made so localhost is added in the list of addresses to open state if we are servingstatus. In that change the addresses where sorted to favor localhost ones, with this new change, if localhost is present, its the only one used and the other ones are discarded. https://code.launchpad.net/~hduran-8/juju-core/apiworker_force_local_only/+merge/221788 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 2

Patch Set 2 : Use localhost only if pressent for opening state #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -69 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M state/api/apiclient.go View 3 chunks +9 lines, -15 lines 0 comments Download
M state/api/apiclient_test.go View 1 4 chunks +11 lines, -54 lines 2 comments Download

Messages

Total messages: 7
hduran
Please take a look.
5 years, 7 months ago (2014-06-02 20:59:54 UTC) #1
thumper
given that localhost should not be in the list more than once, LGTM bonus points ...
5 years, 7 months ago (2014-06-02 21:08:23 UTC) #2
hduran
Please take a look. https://codereview.appspot.com/103820044/diff/1/state/api/apiclient_test.go File state/api/apiclient_test.go (right): https://codereview.appspot.com/103820044/diff/1/state/api/apiclient_test.go#newcode30 state/api/apiclient_test.go:30: listener, err := net.Listen("tcp", "localhost:26104") ...
5 years, 7 months ago (2014-06-03 01:05:00 UTC) #3
thumper
LGTM, just wondering about cleaning up that test. You are good to land if you ...
5 years, 7 months ago (2014-06-03 01:38:51 UTC) #4
hduran
https://codereview.appspot.com/103820044/diff/20001/state/api/apiclient_test.go File state/api/apiclient_test.go (right): https://codereview.appspot.com/103820044/diff/20001/state/api/apiclient_test.go#newcode48 state/api/apiclient_test.go:48: _, port, err := net.SplitHostPort(listenerAddress) On 2014/06/03 01:38:50, thumper ...
5 years, 7 months ago (2014-06-03 01:44:56 UTC) #5
hduran
5 years, 7 months ago (2014-06-03 01:44:58 UTC) #6
jameinel
5 years, 7 months ago (2014-06-03 12:22:55 UTC) #7
LGTM

A comment about "listenAddress contains the actual IP address, but APIHostPorts
is going to report localhost, so just find the port"
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b