Skip to content

Commit

Permalink
Add dial options
Browse files Browse the repository at this point in the history
  • Loading branch information
garyburd committed Aug 29, 2015
1 parent a1be60a commit 5f01fdd
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 34 deletions.
3 changes: 3 additions & 0 deletions internal/redistest/testdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,18 @@ func Dial() (redis.Conn, error) {

_, err = c.Do("SELECT", "9")
if err != nil {
c.Close()
return nil, err
}

n, err := redis.Int(c.Do("DBSIZE"))
if err != nil {
c.Close()
return nil, err
}

if n != 0 {
c.Close()
return nil, errors.New("database #9 is not empty, test can not continue")
}

Expand Down
108 changes: 74 additions & 34 deletions redis/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,56 +51,96 @@ type conn struct {
numScratch [40]byte
}

// Dial connects to the Redis server at the given network and address.
func Dial(network, address string) (Conn, error) {
dialer := xDialer{}
return dialer.Dial(network, address)
}

// DialTimeout acts like Dial but takes timeouts for establishing the
// connection to the server, writing a command and reading a reply.
//
// DialTimeout is deprecated.

This comment has been minimized.

Copy link
@dmitshur

dmitshur Aug 31, 2015

Hi, I have a review comment for this change, I hope you can make use of it to make the code better.

As a personal thought, mentioning that a method is deprecated as the very last word in the documentation seems exact opposite of what it should be - the very first word you see. Otherwise a developer may spend more time reading the entire doc string before realizing this func is deprecated.

Officially speaking, there's supposedly a Go convention already set, see golang/go#10909. However, it's really hard to parse from the linked thread what is the convention that was decided. I hope you're successful in finding it.

This comment has been minimized.

Copy link
@dmitshur

dmitshur Sep 9, 2015

A clear description of the convention is in golang/go#10909 (comment).

func DialTimeout(network, address string, connectTimeout, readTimeout, writeTimeout time.Duration) (Conn, error) {
netDialer := net.Dialer{Timeout: connectTimeout}
dialer := xDialer{
NetDial: netDialer.Dial,
ReadTimeout: readTimeout,
WriteTimeout: writeTimeout,
}
return dialer.Dial(network, address)
return Dial(network, address,
DialConnectTimeout(connectTimeout),
DialReadTimeout(readTimeout),
DialWriteTimeout(writeTimeout))
}

// A Dialer specifies options for connecting to a Redis server.
type xDialer struct {
// NetDial specifies the dial function for creating TCP connections. If
// NetDial is nil, then net.Dial is used.
NetDial func(network, addr string) (net.Conn, error)
// DialOption specifies an option for dialing a Redis server.
type DialOption struct {
f func(*dialOptions)
}

// ReadTimeout specifies the timeout for reading a single command
// reply. If ReadTimeout is zero, then no timeout is used.
ReadTimeout time.Duration
type dialOptions struct {
readTimeout time.Duration
writeTimeout time.Duration
dial func(network, addr string) (net.Conn, error)
db int
password string
}

// DialReadTimeout specifies the timeout for reading a single command reply.
func DialReadTimeout(d time.Duration) DialOption {
return DialOption{func(do *dialOptions) {
do.readTimeout = d
}}
}

// DialWriteTimeout specifies the timeout for writing a single command.
func DialWriteTimeout(d time.Duration) DialOption {
return DialOption{func(do *dialOptions) {
do.writeTimeout = d
}}
}

// DialConnectTimeout specifies the timeout for connecting to the Redis server.
func DialConnectTimeout(d time.Duration) DialOption {
return DialOption{func(do *dialOptions) {
dialer := net.Dialer{Timeout: d}
do.dial = dialer.Dial
}}
}

// WriteTimeout specifies the timeout for writing a single command. If
// WriteTimeout is zero, then no timeout is used.
WriteTimeout time.Duration
// DialDatabase specifies the database to select when dialing a connection.
func DialDatabase(db int) DialOption {
return DialOption{func(do *dialOptions) {
do.db = db
}}
}

// Dial connects to the Redis server at address on the named network.
func (d *xDialer) Dial(network, address string) (Conn, error) {
dial := d.NetDial
if dial == nil {
dial = net.Dial
// Dial connects to the Redis server at the given network and
// address using the specified options.
func Dial(network, address string, options ...DialOption) (Conn, error) {
do := dialOptions{
dial: net.Dial,
}
netConn, err := dial(network, address)
for _, option := range options {
option.f(&do)
}

netConn, err := do.dial(network, address)
if err != nil {
return nil, err
}
return &conn{
c := &conn{
conn: netConn,
bw: bufio.NewWriter(netConn),
br: bufio.NewReader(netConn),
readTimeout: d.ReadTimeout,
writeTimeout: d.WriteTimeout,
}, nil
readTimeout: do.readTimeout,
writeTimeout: do.writeTimeout,
}

if do.password != "" {
if _, err := c.Do("AUTH", do.password); err != nil {
netConn.Close()
return nil, err
}
}

if do.db != 0 {
if _, err := c.Do("SELECT", do.db); err != nil {
netConn.Close()
return nil, err
}
}

return c, nil
}

// NewConn returns a new Redigo connection for the given net connection.
Expand Down

0 comments on commit 5f01fdd

Please sign in to comment.