go-zookeeper / zk

Native ZooKeeper client for Go
BSD 3-Clause "New" or "Revised" License
506 stars 130 forks source link

conn.Multi should take a specific interface and not `interface{}` #3

Open nemith opened 5 years ago

nemith commented 5 years ago

conn.Multi signature is func (c *Conn) Multi(ops ...interface{}) ([]MultiResponse, error) where ops is any type. However it will return an error if it is not one of CreateRequest, SetDataRequest, DeleteRequest, or CheckVersionRequest:

We could implement a interface that all these structs implement to limit to just the operations from the package. This could just return the opCode itself.

type operation interface {
  opCode() int32  // better yet type opCodes as well.
}
yarikk commented 4 years ago

Since this would be an incompatible change, the milestone's got to be v2.

nemith commented 4 years ago

This should be compatible since the function cannot just take anything we would just enforce it at compile time instead of runtime.

nemith commented 4 years ago

However looking at this further these really shouldn't take the packet definitions as options. As we add features like TTL there are two different packet definitions that the API consumer doesn't need to know about.