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

Issue 83500043: Introduce worker/apiaddressupdater

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 1 month ago by axw
Modified:
10 years, 1 month ago
Reviewers:
mp+213778, rog
Visibility:
Public.

Description

Introduce worker/apiaddressupdater This proposal introduces a new worker that watches changes to APIHostPorts, and then updates agent config. The machine and unit agents have both been updated to start this worker. https://code.launchpad.net/~axwalk/juju-core/agent-watch-apihostports/+merge/213778 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 6

Patch Set 2 : Introduce worker/apiaddressupdater #

Unified diffs Side-by-side diffs Delta from patch set Stats (+232 lines, -0 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/jujud/agent.go View 2 chunks +8 lines, -0 lines 0 comments Download
M cmd/jujud/machine.go View 2 chunks +4 lines, -0 lines 0 comments Download
M cmd/jujud/machine_test.go View 1 1 chunk +26 lines, -0 lines 0 comments Download
M cmd/jujud/unit.go View 2 chunks +4 lines, -0 lines 0 comments Download
M cmd/jujud/unit_test.go View 1 3 chunks +27 lines, -0 lines 0 comments Download
A worker/apiaddressupdater/apiaddressupdater.go View 1 chunk +64 lines, -0 lines 0 comments Download
A worker/apiaddressupdater/apiaddressupdater_test.go View 1 chunk +97 lines, -0 lines 0 comments Download

Messages

Total messages: 3
axw
Please take a look.
10 years, 1 month ago (2014-04-02 08:43:41 UTC) #1
rog
LGTM with a few queries below, thanks! https://codereview.appspot.com/83500043/diff/1/cmd/jujud/machine_test.go File cmd/jujud/machine_test.go (right): https://codereview.appspot.com/83500043/diff/1/cmd/jujud/machine_test.go#newcode851 cmd/jujud/machine_test.go:851: s.State.StartSync() shouldn't ...
10 years, 1 month ago (2014-04-02 08:51:08 UTC) #2
axw
10 years, 1 month ago (2014-04-02 09:49:00 UTC) #3
Please take a look.

https://codereview.appspot.com/83500043/diff/1/cmd/jujud/machine_test.go
File cmd/jujud/machine_test.go (right):

https://codereview.appspot.com/83500043/diff/1/cmd/jujud/machine_test.go#newc...
cmd/jujud/machine_test.go:851: s.State.StartSync()
On 2014/04/02 08:51:08, rog wrote:
> shouldn't this be BackingState? (i think the watcher is actually in the API,
> right?)

Done.

https://codereview.appspot.com/83500043/diff/1/cmd/jujud/machine_test.go#newc...
cmd/jujud/machine_test.go:852: timeout := time.After(coretesting.LongWait)
On 2014/04/02 08:51:08, rog wrote:
> perhaps this might be better phrased as an attempt loop?

Done, much nicer.

https://codereview.appspot.com/83500043/diff/1/cmd/jujud/unit_test.go
File cmd/jujud/unit_test.go (right):

https://codereview.appspot.com/83500043/diff/1/cmd/jujud/unit_test.go#newcode270
cmd/jujud/unit_test.go:270: s.State.StartSync()
On 2014/04/02 08:51:08, rog wrote:
> shouldn't this be BackingState?

Done.
Sign in to reply to this message.

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