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

Issue 6441047: add RelationState type...

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 8 months ago by fwereade
Modified:
11 years, 8 months ago
Reviewers:
mp+116697
Visibility:
Public.

Description

add RelationState type... ...and use it to reconcile HookQueue state with initial watcher event on load. As part of this, I added a RelationId to HookQueue, and return it in every HookInfo, so that we can identify what relation the hook is actually meant to run against. https://code.launchpad.net/~fwereade/juju-core/hook-queue-state/+merge/116697 Requires: https://code.launchpad.net/~fwereade/juju-core/hook-queue-watcher-style/+merge/116684 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : add QueueState type... #

Patch Set 3 : add QueueState type... #

Patch Set 4 : add QueueState type... #

Patch Set 5 : add RelationState type... #

Total comments: 10

Patch Set 6 : add RelationState type... #

Unified diffs Side-by-side diffs Delta from patch set Stats (+156 lines, -42 lines) Patch
A [revision details] View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M worker/uniter/hookqueue.go View 1 2 3 4 5 3 chunks +76 lines, -21 lines 0 comments Download
M worker/uniter/hookqueue_test.go View 1 2 3 4 5 8 chunks +78 lines, -21 lines 0 comments Download

Messages

Total messages: 6
fwereade
Please take a look.
11 years, 8 months ago (2012-07-25 16:46:51 UTC) #1
fwereade
Please take a look.
11 years, 8 months ago (2012-07-26 16:48:34 UTC) #2
fwereade
Please take a look.
11 years, 8 months ago (2012-07-27 08:28:48 UTC) #3
niemeyer
This is brilliant. https://codereview.appspot.com/6441047/diff/4006/worker/uniter/hookqueue.go File worker/uniter/hookqueue.go (right): https://codereview.appspot.com/6441047/diff/4006/worker/uniter/hookqueue.go#newcode20 worker/uniter/hookqueue.go:20: type HookInfo struct { // RelationId ...
11 years, 8 months ago (2012-07-27 15:54:20 UTC) #4
niemeyer
LGTM
11 years, 8 months ago (2012-07-27 15:54:34 UTC) #5
fwereade
11 years, 8 months ago (2012-07-27 16:33:23 UTC) #6
*** Submitted:

add RelationState type...

...and use it to reconcile HookQueue state with initial watcher event on
load.

As part of this, I added a RelationId to HookQueue, and return it in every
HookInfo, so that we can identify what relation the hook is actually meant
to run against.

R=niemeyer
CC=
https://codereview.appspot.com/6441047

https://codereview.appspot.com/6441047/diff/4006/worker/uniter/hookqueue.go
File worker/uniter/hookqueue.go (right):

https://codereview.appspot.com/6441047/diff/4006/worker/uniter/hookqueue.go#n...
worker/uniter/hookqueue.go:20: type HookInfo struct {
On 2012/07/27 15:54:20, niemeyer wrote:
> // RelationId identifies the relation associated with the hook queue.
> RelationId int
> 
> // HookKind is one of "joined", "changed", or "departed".
> HookKind string
> 
> // RemoteUnit is the unit name associated with HookKind.
> RemoteUnit string
> 
> // ChangeVersion identifies the unit settings change that motivated
> // the HookInfo to be sent to the output channel.
> ChangeVersion int
> 
> // Members is a map from remote unit name to its settings.
> // It contains all members known in the relation up to the
> // moment in which the HookInfo was delivered.
> Members map[string]map[string]interface{}

Done.

https://codereview.appspot.com/6441047/diff/4006/worker/uniter/hookqueue.go#n...
worker/uniter/hookqueue.go:32: // and that communicated by its underlying
watcher.
On 2012/07/27 15:54:20, niemeyer wrote:
> s/RelationState/HookQueueState/, with doc:
> 
> // HookQueueState is a snapshot of the state of a HookQueue.
> 
> (the rest is documentation for NewHookQueue)

As discussed live, this type will be the input data for both a HookQueue and the
RelationSettings(?) type that will be fed data by the HookQueue via the
Uniter(?). It'll move to its own file soon enough, but it doesn't deserve it
yet.

https://codereview.appspot.com/6441047/diff/4006/worker/uniter/hookqueue.go#n...
worker/uniter/hookqueue.go:33: type RelationState struct {
On 2012/07/27 15:54:20, niemeyer wrote:
> // RelationId identifies the relation associated with the HookQueue.
> RelationId int
> 
> // Members is a map from unit name to the last change version
> // for which a HookInfo was delivered on the output channel.
> Members map[string]int
> 
> // PendingChanged indicates that a "changed" hook for the given unit
> // name is the first HookInfo to be sent to the output channel.
> PendingChanged string

Done.

https://codereview.appspot.com/6441047/diff/4006/worker/uniter/hookqueue_test.go
File worker/uniter/hookqueue_test.go (right):

https://codereview.appspot.com/6441047/diff/4006/worker/uniter/hookqueue_test...
worker/uniter/hookqueue_test.go:37: ), nilTest(
On 2012/07/27 15:54:20, niemeyer wrote:
> I was a bit confused by the nilTest name. It's definitely not a *nil* *test*.
I
> suggest calling the one with nil a fullTest and the other a reconcileTest, or
> something else you prefer that gives that feeling.

Done.
Sign in to reply to this message.

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