Some concerns with DialOpt

[Taru Karttunen]

Hello

I am wondering whether DialOpt is a very good fit for the
requirement of Dialing with options.

How do third-party packages deal with DialOptions?
e.g. how would my dialer get the net.Network from
a []DialOption? Just passing it through is not a solution
(e.g. "dial tcp6 to example.com" -> resolve the domain
name in a foreign proxy and tell the proxy to use ipv6).

How does one marshal+unmarshal DialOptions? (e.g. json)

Would it be possible to have DialOptions that are
working from third party code?

- Taru Karttunen

[Julien Schmidt]

I don't think there is a nice solution for this with the current approach.
There was also a struct version discussed, which I'd still favor: https://codereview.appspot.com/7365049
I'm not happy with the current approach and think this leads to unnecessary complexity and problems.

And once it is in the Go 1.1 API the API contract forces us to deal with it until Go 2.

[Ingo Oeser]

I have more love for packages that accept an io.ReadWriteCloser instead of have their own Dial method/function.

That makes it much easier to write tests for it, also for the package author itself.

It neatly solves most of these problems by providing an orthogonal API.

An *additional* TCP dialer for convenience is no mistake.

[Ingo Oeser]

[roger peppe]

I hadn't looked at this until now and I tend to agree with your concerns.
The DialOption thing seems a bit too clever for its own good.
Placing the options behind an opaque interface barrier doesn't
look great to me.

I'm pretty sure it's too late now though - this will go into 1.1.

There is an external workaround. You can define a package that really
does define such a struct, and a method on it that converts
to dial options.

type DialOpts struct {
Deadline time.Time
Network string
etc
}

func (d *DialOpts) Opts() []net.DialOption {
var opts []net.DialOption
if !d.Deadline.IsZero() {
opts = append(opts, net.Deadline(d.Deadline))
}
if d.Network != "" {
opts = append(opts, net.Network(d.Network))
}
... etc
return opts
}

As long as you pass around the struct and not []net.DialOption,
you can do all you need to (with some allocation cost per dial)

One advantage of net.DialOpt is that there's no
need to assume that the zero value of a field implies the default.
I'm not sure that's a hugely strong strong argument though - net
uses a struct under the hood anyway as it happens.

> --
> You received this message because you are subscribed to the Google Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[Anthony Martin]

I agree.

It would've been nice if the API conformed to the
code.google.com/p/go.net/proxy.Dialer interface.

Something like:

type DialOpts struct {
Deadline time.Time
Network string

LocalAddress Addr
}

func (d *DialOpts) Dial(net, addr string) (Conn, error)

As it stands now, you have to add an extra closure
or top-level function to build a Dialer.

Anthony

[Kyle Lemons]

On Thu, Mar 28, 2013 at 3:53 AM, roger peppe <rogp...@gmail.com> wrote:

I hadn't looked at this until now and I tend to agree with your concerns.
The DialOption thing seems a bit too clever for its own good.
Placing the options behind an opaque interface barrier doesn't
look great to me.

I had the same thought initially, until I went and read through the various comments in the CLs.  A struct-based approach was definitely considered.

 

I'm pretty sure it's too late now though - this will go into 1.1.

There is an external workaround. You can define a package that really
does define such a struct, and a method on it that converts
to dial options.

type DialOpts struct {
     Deadline time.Time
     Network string
     etc
}

This is almost precisely what the implementation is on the inside.

 

func (d *DialOpts) Opts() []net.DialOption {
    var opts []net.DialOption
    if !d.Deadline.IsZero() {
        opts = append(opts, net.Deadline(d.Deadline))
    }
    if d.Network != "" {
        opts = append(opts, net.Network(d.Network))
    }
    ... etc
   return opts
}

As long as you pass around the struct and not []net.DialOption,
you can do all you need to (with some allocation cost per dial)

One advantage of net.DialOpt is that there's no
need to assume that the zero value of a field implies the default.
I'm not sure that's a hugely strong strong argument though - net
uses a struct under the hood anyway as it happens.

(disclaimer: the following is purely my opinion and may not reflect the reality of the plans for this API)

I see the DialOpt interface as the seed of an API to come.  While it's currently opaque and third party packages cannot implement it, I don't think that it is intended to stay that way forever.  When we're able to design a function or a few functions that encapsulate the kinds of things that need to be done before/after the dial, we can implement them on the existing DialOpt implementations and make them exported, at which point third party packages could be able to do things like provide a DialOpt that sets SO_LINGER or SO_OOBINLINE or any other random option that's unlikely to ever wind up in the net library.  It could potentially even provide a DialOpt that completely changes the way the socket is dialed, to e.g. proxy the data over some other medium, etc.  It could allow for DialOpts that understand special addressing modes, load balancing, failover, timeouts, etc.  Who knows.  The point is that no third party can teach the net library how to do any of this if the behavior is limited to setting fields in a DialOption struct, and if it's something that requires low-level access to the socket before it's put into the net library's poll server, it might be impossible to do as a third party otherwise (without duplicating net).

[Brad Fitzpatrick]

[+golang-dev]

Opinions wanted.  See below.

You, Taru, and others in this thread make some good points.

Using private interfaces and varargs is cute, but does prevent re-use in other packages.

To summarize for golang-dev:

-- net.Dial takes no options. this is bad.

-- net.DialTimeout only takes a timeout. this fixed a top complaint, but wasn't extensible enough. sorry.

-- there are many more options, for better or worse. (queue Rob's love for sockets)

-- in Go 1.0, we had no way to add new dial options.

-- in Go 1.1, we do want something that grows nicely in the future.

-- we don't want an interface like:

        net.DialStuff("tcp", "google.com:80", nil, nil, 0, nil, false, time.Time{}, nil)

   or even:

        net.DialOpt("google.com:80", nil)   // nil? gross. what is that?

-- whatever we do, we want to be able to add more options in Go 1.2 without changing

   too much.  Adding fields to a struct is fine. Adding new funcs to return DIalOptions is fine.

   Adding new types is starting to get gross. Adding new Dial funcs is off the table.

-- today in tip: net.DialOption is an interface with only private methods.

-- today in tip: all the functions to create options only return net.DialOption.

-- today in tip: it's hard for third-party packages to take a net.DialOption and do anything with it

   except pass it through unchanged. (for better or worse. for better: they can't forget to respect

   options. for worse: they can't respect options, nor test)

-- go.net contains a Dialer interface: http://godoc.org/code.google.com/p/go.net/proxy#Dialer

   type Dialer interface {

    // Dial connects to the given address via the proxy.

    Dial(network, addr string) (c net.Conn, err error)

    }

-- net/http.Transport contains an optional Dial func hook: http://golang.org/pkg/net/http/#Transport

        // Dial specifies the dial function for creating TCP

        // connections.

        // If Dial is nil, net.Dial is used.

        Dial func(net, addr string) (c net.Conn, err error)

-- we now have method values in tip (to be Go 1.1): http://tip.golang.org/ref/spec#Method_values

One of the earliest Dial option proposals was a struct of options.  That's still an option, and is slightly more attractive with method values now.

We could do:

package net

// Dialer specifies the parameters for making a network connection.

// The zero value is a valid Dialer. 

type Dialer struct {

      Timeout time.Duration

      Deadline time.Time

      LocalAddr net.Addr

      ....

      FastOption bool // (later, in Go 1.2)

}

func (d *Dialer) Dial func(net, addr string) (net.Conn, error)

And then you could say:

      dialer := &net.Dialer{

             LocalAddr: addr,

             Timeout: 15 * time.Second,

      }

      dialer.Dial("tcp", "google.com:80")

      // or:

      tr := &http.Transport{

           Dial: dialer.Dial,

      }

      // or

      wantsAnInterface(dialer)

That also seems acceptable.

Does anybody want to argue for or against either way?

We could still change it, but not for long.

[Taru Karttunen]

On 01.04 16:24, Brad Fitzpatrick wrote:
> [+golang-dev]
>
> Opinions wanted. See below.

I would like to advocate having dialing with options not marked as
API stable and have it experimental for Go 1.1. Is this possible
from the API perspective? This would give time to find an API
that works for various concerns.

I do advocate a struct if this is not possible.

Additionally with the struct one could do e.g:

package tor

type Dialer struct {
net.Dialer
NoConnectionReuse bool
}

func (d *Dialer)Dial(network string, addr string) (net.Conn, error) {
...
}

Also various defaults can be supported:

var mydialerA = &Dialer{ ... }
var mydialerB = &Dialer{ ... }

tls:

package tls

type Dialer struct {
net.Dialer
Options
}

func (*Dialer)Dial ...

Marshalling & serializing? Of course. Low level options from syscall?

package net

type Dialer struct {
...
Syscall *syscall.NetOptions
}

etc

Marshalling&Unmarshalling? No problem. Extending things? No problem
either. Passing to e.g. net/http? No problem with method expressions.

- Taru Karttunen

[Brad Fitzpatrick]

On Mon, Apr 1, 2013 at 11:20 PM, Taru Karttunen <tar...@taruti.net> wrote:

On 01.04 16:24, Brad Fitzpatrick wrote:
> [+golang-dev]
>
> Opinions wanted.  See below.

I would like to advocate having dialing with options not marked as
API stable and have it experimental for Go 1.1. Is this possible
from the API perspective?

No.  We need to pick a color before Go 1.1.

 

I do advocate a struct if this is not possible.

Additionally with the struct one could do e.g:

package tor

type Dialer struct {
    net.Dialer
    NoConnectionReuse bool
}

That's another good argument for structs, thanks.

[roger peppe]

I'm of the same opinion I was before - I think a struct is a nicer
and more idiomatic solution. If we can change it before 1.1, I'll
be happy.

[Julien Schmidt]

As long we find a solution based on an exported struct, I'm happy with that.

Passing the struct as an parameter to the dail function would be more consistent with the existing functions. But since the long term plan seems to be the removal of the two existing functions, I favor the dail method on the DailOpts struct, which feels very "go-ish".

[Brad Fitzpatrick]

Mailed https://codereview.appspot.com/8274043/

[roger peppe]

On 2 April 2013 20:25, Brad Fitzpatrick <brad...@golang.org> wrote:
> Mailed https://codereview.appspot.com/8274043/

wonderful, thanks!