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

Issue 5672067: Presence nodes

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 1 month ago by fwereade
Modified:
12 years ago
Reviewers:
mp+93357
Visibility:
Public.

Description

A replacement for zookeeper ephemeral nodes, which allows for seamless reestablishment of presence across separate sessions. https://code.launchpad.net/~fwereade/juju/go-presence-nodes/+merge/93357 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 11

Patch Set 2 : Ephemeral node replacement sketch, DON'T SUBMIT #

Patch Set 3 : Ephemeral node replacement sketch #

Total comments: 5

Patch Set 4 : Presence nodes #

Patch Set 5 : Presence nodes #

Total comments: 32

Patch Set 6 : Presence nodes #

Total comments: 4

Patch Set 7 : Presence nodes #

Patch Set 8 : Presence nodes #

Total comments: 8

Patch Set 9 : Presence nodes #

Total comments: 52

Patch Set 10 : Presence nodes #

Total comments: 4

Patch Set 11 : Presence nodes #

Total comments: 1

Patch Set 12 : Presence nodes #

Total comments: 18

Patch Set 13 : Presence nodes #

Patch Set 14 : Presence nodes #

Patch Set 15 : Presence nodes #

Unified diffs Side-by-side diffs Delta from patch set Stats (+535 lines, -0 lines) Patch
A state/presence/presence.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +243 lines, -0 lines 0 comments Download
A state/presence/presence_test.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +292 lines, -0 lines 0 comments Download

Messages

Total messages: 30
fwereade
Please take a look.
12 years, 1 month ago (2012-02-16 09:08:19 UTC) #1
rog
i really like the essential idea (OccupiedW). could do with a bit of refinement though. ...
12 years, 1 month ago (2012-02-16 12:00:36 UTC) #2
fwereade
Please take a look.
12 years, 1 month ago (2012-02-17 18:28:52 UTC) #3
fwereade
Please take a look.
12 years, 1 month ago (2012-02-20 18:26:38 UTC) #4
rog
as discussed on IRC, i don't think this API is an improvement. in particular the ...
12 years, 1 month ago (2012-02-21 10:25:47 UTC) #5
fwereade
Please take a look.
12 years, 1 month ago (2012-02-23 13:30:50 UTC) #6
fwereade
Please take a look.
12 years, 1 month ago (2012-02-23 13:58:24 UTC) #7
rog
lots of comments again, sorry. it's looking good in general though. the underlying logic looks ...
12 years, 1 month ago (2012-02-23 15:30:29 UTC) #8
fwereade
Please take a look.
12 years, 1 month ago (2012-02-24 14:12:37 UTC) #9
fwereade
Please take a look.
12 years, 1 month ago (2012-02-25 09:14:07 UTC) #10
fwereade
Please take a look.
12 years, 1 month ago (2012-02-25 16:27:55 UTC) #11
fwereade
These may be somewhat redundant now, given the changes in the following patch sets, but ...
12 years, 1 month ago (2012-02-27 09:57:16 UTC) #12
fwereade
Please take a look.
12 years, 1 month ago (2012-02-27 10:21:58 UTC) #13
rog
i think this is very nearly there. https://codereview.appspot.com/5672067/diff/12004/state/presence/presence.go File state/presence/presence.go (right): https://codereview.appspot.com/5672067/diff/12004/state/presence/presence.go#newcode113 state/presence/presence.go:113: n.timeout = ...
12 years, 1 month ago (2012-02-27 10:45:52 UTC) #14
niemeyer
This is looking very good. There's only one issue worth of attention, and then mostly ...
12 years, 1 month ago (2012-02-27 14:50:34 UTC) #15
niemeyer
https://codereview.appspot.com/5672067/diff/10004/state/presence/presence.go File state/presence/presence.go (right): https://codereview.appspot.com/5672067/diff/10004/state/presence/presence.go#newcode61 state/presence/presence.go:61: func (p *Pinger) Close() { On 2012/02/27 14:50:34, niemeyer ...
12 years, 1 month ago (2012-02-27 14:58:43 UTC) #16
fwereade
Please take a look.
12 years, 1 month ago (2012-02-28 09:30:08 UTC) #17
rog
LGTM https://codereview.appspot.com/5672067/diff/18001/state/presence/presence.go File state/presence/presence.go (right): https://codereview.appspot.com/5672067/diff/18001/state/presence/presence.go#newcode99 state/presence/presence.go:99: // ie there's no way it can be ...
12 years, 1 month ago (2012-02-28 09:47:37 UTC) #18
niemeyer
https://codereview.appspot.com/5672067/diff/18001/state/presence/presence.go File state/presence/presence.go (right): https://codereview.appspot.com/5672067/diff/18001/state/presence/presence.go#newcode108 state/presence/presence.go:108: period, err := time.ParseDuration(content) You have the new content ...
12 years, 1 month ago (2012-02-28 14:53:59 UTC) #19
fwereade
Whoops, this is at least 3 reviews' responses backed up... sorry :(. https://codereview.appspot.com/5672067/diff/12004/state/presence/presence.go File state/presence/presence.go ...
12 years, 1 month ago (2012-02-28 16:31:00 UTC) #20
fwereade
Please take a look.
12 years, 1 month ago (2012-02-28 16:33:59 UTC) #21
rog
https://codereview.appspot.com/5672067/diff/22001/state/presence/presence.go File state/presence/presence.go (right): https://codereview.appspot.com/5672067/diff/22001/state/presence/presence.go#newcode97 state/presence/presence.go:97: timeoutWasSet := n.timeout != 0 this seems a little ...
12 years, 1 month ago (2012-02-28 16:43:22 UTC) #22
fwereade
Please take a look.
12 years, 1 month ago (2012-02-28 17:04:03 UTC) #23
rog
https://codereview.appspot.com/5672067/diff/24001/state/presence/presence.go File state/presence/presence.go (right): https://codereview.appspot.com/5672067/diff/24001/state/presence/presence.go#newcode93 state/presence/presence.go:93: // content and stat of the zookeeper node (which ...
12 years, 1 month ago (2012-02-28 17:12:49 UTC) #24
niemeyer
LGTM.. just a few trivials. https://codereview.appspot.com/5672067/diff/24001/state/presence/presence.go File state/presence/presence.go (right): https://codereview.appspot.com/5672067/diff/24001/state/presence/presence.go#newcode98 state/presence/presence.go:98: return fmt.Errorf("%s is not ...
12 years ago (2012-03-08 21:58:45 UTC) #25
fwereade
Please take a look.
12 years ago (2012-03-08 22:06:31 UTC) #26
fwereade
https://codereview.appspot.com/5672067/diff/24001/state/presence/presence.go File state/presence/presence.go (right): https://codereview.appspot.com/5672067/diff/24001/state/presence/presence.go#newcode93 state/presence/presence.go:93: // content and stat of the zookeeper node (which ...
12 years ago (2012-03-08 22:53:57 UTC) #27
fwereade
Please take a look.
12 years ago (2012-03-08 22:55:05 UTC) #28
niemeyer
LGTM, thanks!
12 years ago (2012-03-09 08:27:53 UTC) #29
fwereade
12 years ago (2012-03-09 08:55:08 UTC) #30
*** Submitted:

Presence nodes

A replacement for zookeeper ephemeral nodes, which allows for seamless
reestablishment of presence across separate sessions.

R=rog, niemeyer
CC=
https://codereview.appspot.com/5672067
Sign in to reply to this message.

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