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

Issue 6524048: add Settings and ReadSettings to RelationUnit

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

Description

add Settings and ReadSettings to RelationUnit https://code.launchpad.net/~fwereade/juju-core/mstate-relation-unit-settings/+merge/125151 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 6

Patch Set 2 : add Settings and ReadSettings to RelationUnit #

Unified diffs Side-by-side diffs Delta from patch set Stats (+240 lines, -12 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M mstate/relation.go View 1 4 chunks +75 lines, -12 lines 0 comments Download
M mstate/relation_test.go View 2 chunks +163 lines, -0 lines 0 comments Download

Messages

Total messages: 3
fwereade
Please take a look.
11 years, 6 months ago (2012-09-19 10:08:28 UTC) #1
niemeyer
LGTM, assuming we agree on something for the key. https://codereview.appspot.com/6524048/diff/1/mstate/relation.go File mstate/relation.go (right): https://codereview.appspot.com/6524048/diff/1/mstate/relation.go#newcode224 mstate/relation.go:224: ...
11 years, 6 months ago (2012-09-19 11:39:06 UTC) #2
fwereade
11 years, 6 months ago (2012-09-19 13:40:40 UTC) #3
*** Submitted:

add Settings and ReadSettings to RelationUnit

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

https://codereview.appspot.com/6524048/diff/1/mstate/relation.go
File mstate/relation.go (right):

https://codereview.appspot.com/6524048/diff/1/mstate/relation.go#newcode224
mstate/relation.go:224: key, err := ru.key(ru.unit.Name())
On 2012/09/19 11:39:06, niemeyer wrote:
> Can we please talk about this key live? We need to scope it to isolate from
the
> other settings-holders as well, since we're using it as a key on the settings
> collection.

Agreed "r#" prefix live. Done.

https://codereview.appspot.com/6524048/diff/1/mstate/relation.go#newcode243
mstate/relation.go:243: }
On 2012/09/19 11:39:06, niemeyer wrote:
> // TODO Drop the Count once readConfigNode refuses to read
> // non-existent settings (which it should).

Done.

https://codereview.appspot.com/6524048/diff/1/mstate/relation.go#newcode244
mstate/relation.go:244: if n, err := ru.st.settings.Find(D{{"_id",
key}}).Count(); err != nil {
On 2012/09/19 11:39:06, niemeyer wrote:
> This can be FindId(key)

Done.
Sign in to reply to this message.

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