Merge lp://staging/~rogpeppe/gozk/comments-and-time into lp://staging/gozk/zookeeper

Reviewers: mp+92750_code.launchpad.net,

Message:
Please take a look.

Description:
Also change Stat.CTime and Stat.MTime to return time.Time.

https://code.launchpad.net/~rogpeppe/gozk/comments-and-time/+merge/92750

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/5650075/

Affected files:
   M zk.go
   M zk_test.go

Index: zk.go
=== <email address hidden> >
<email address hidden>
=== modified file 'zk.go'
--- zk.go 2012-02-08 14:34:24 +0000
+++ zk.go 2012-02-13 12:38:16 +0000
@@ -92,8 +92,17 @@
  // event.Type is set to EVENT_CLOSED and event.State is set to
STATE_CLOSED,
  // to facilitate handling.
  type Event struct {
- Type int
- Path string
+ // Type gives the type of event (one of the EVENT_* constants).
+ // If Type is EVENT_SESSION, then the event is a session
+ // event.
+ Type int
+
+ // For non-session events, Path gives the path of the node
+ // that was being watched.
+ Path string
+
+ // For session events, State (one of the STATE* constants) gives the
session
+ // status.
   State int
  }

@@ -304,46 +313,64 @@
   c C.struct_Stat
  }

+// Czxid returns the zxid of the change that caused the node to be created.
  func (stat *Stat) Czxid() int64 {
   return int64(stat.c.czxid)
  }

+// Mzxid returns the zxid of the change that last modified the node.
  func (stat *Stat) Mzxid() int64 {
   return int64(stat.c.mzxid)
  }

-func (stat *Stat) CTime() int64 {
- return int64(stat.c.ctime)
-}
-
-func (stat *Stat) MTime() int64 {
- return int64(stat.c.mtime)
-}
-
+func millisec2time(ms int64) time.Time {
+ return time.Unix(ms/1e3, ms%1e3*1e6)
+}
+
+// CTime returns the time (at millisecond resolution) when the node was
+// created.
+func (stat *Stat) CTime() time.Time {
+ return millisec2time(int64(stat.c.ctime))
+}
+
+// MTime returns the time (at millisecond resolution) when the node was
+// last modified.
+func (stat *Stat) MTime() time.Time {
+ return millisec2time(int64(stat.c.mtime))
+}
+
+// Version returns the number of changes to the data of the node.
  func (stat *Stat) Version() int32 {
   return int32(stat.c.version)
  }

+// CVersion returns the number of changes to the children of the node.
  func (stat *Stat) CVersion() int32 {
   return int32(stat.c.cversion)
  }

+// AVersion returns the number of changes to the ACL of the node.
  func (stat *Stat) AVersion() int32 {
   return int32(stat.c.aversion)
  }

+// If the node is an ephemeral node, EphemeralOwner returns the session id
+// of the owner of the node; otherwise it will return zero.
  func (stat *Stat) EphemeralOwner() int64 {
   return int64(stat.c.ephemeralOwner)
  }

+// DataLength returns the length of the data in the node in bytes.
  func (stat *Stat) DataLength() int32 {
   return int32(stat.c.dataLength)
  }

+// NumChildren returns the number of children of the znode.
  func (stat *Stat) NumChildren() int32 {
   return int32(stat.c.numChildren)
  }

+// Pzxid returns the Pzxid of the node, whatever that is.
  func (stat *Stat) Pzxid() int64 {
   return int64(stat.c.pzxid)
  }

Index: zk_test.go
=== roger.peppe@canonical....

https://codereview.appspot.com/5650075/diff/1/zk.go
File zk.go (right):

https://codereview.appspot.com/5650075/diff/1/zk.go#newcode97
zk.go:97: // event.
This is extremely redundant. Please use something along these lines
instead:

type Event struct {
         Type int // One of the EVENT_* constants
         Path string // Node path for non-session events
         State int // One of the STATE_* constants
}

Note that State exists for non-session events also.

https://codereview.appspot.com/5650075/diff/1/zk.go#newcode347
zk.go:347: // CVersion returns the number of changes to the children of
the node.
This needs clarification. Will a change in a child's data increase this
version too?

https://codereview.appspot.com/5650075/diff/1/zk.go#newcode368
zk.go:368: // NumChildren returns the number of children of the znode.
s/znode/node/, as everywhere else.

https://codereview.appspot.com/5650075/diff/1/zk.go#newcode373
zk.go:373: // Pzxid returns the Pzxid of the node, whatever that is.
:-)

https://codereview.appspot.com/5650075/diff/1/zk_test.go
File zk_test.go (right):

https://codereview.appspot.com/5650075/diff/1/zk_test.go#newcode187
zk_test.go:187: if dt < 0 || dt > maxdt {
if t.Before(t0) || t.After(t1) {

https://codereview.appspot.com/5650075/

Please take a look.

https://codereview.appspot.com/5650075/

PTAL

https://codereview.appspot.com/5650075/diff/1/zk.go
File zk.go (right):

https://codereview.appspot.com/5650075/diff/1/zk.go#newcode97
zk.go:97: // event.
On 2012/02/13 13:53:57, niemeyer wrote:
> This is extremely redundant. Please use something along these lines
instead:

> type Event struct {
> Type int // One of the EVENT_* constants
> Path string // Node path for non-session events
> State int // One of the STATE_* constants
> }

> Note that State exists for non-session events also.

Done.

https://codereview.appspot.com/5650075/diff/1/zk.go#newcode347
zk.go:347: // CVersion returns the number of changes to the children of
the node.
On 2012/02/13 13:53:57, niemeyer wrote:
> This needs clarification. Will a change in a child's data increase
this version
> too?

Done.

https://codereview.appspot.com/5650075/diff/1/zk.go#newcode368
zk.go:368: // NumChildren returns the number of children of the znode.
On 2012/02/13 13:53:57, niemeyer wrote:
> s/znode/node/, as everywhere else.

Done.

https://codereview.appspot.com/5650075/diff/1/zk_test.go
File zk_test.go (right):

https://codereview.appspot.com/5650075/diff/1/zk_test.go#newcode187
zk_test.go:187: if dt < 0 || dt > maxdt {
On 2012/02/13 13:53:57, niemeyer wrote:
> if t.Before(t0) || t.After(t1) {

doh!

https://codereview.appspot.com/5650075/

LGTM

https://codereview.appspot.com/5650075/

*** Submitted:

Add some comments to Event and Stat types.

Also change Stat.CTime and Stat.MTime to return time.Time.

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

https://codereview.appspot.com/5650075/