support similar connect options as other clients

[fmoor]

This change adopts a connect API similar to the java script client accepting a dsn string and an options object.

fixes #22

[1st1]

Overall looks good. I'm wondering, did you fully replicate the functionality/logic/tests from edgedb-python & edgedb-js?

[fmoor]

did you fully replicate the functionality/logic/tests from edgedb-python & edgedb-js?

I believe that I have fully replicated the logic for parsing connect arguments and trying each address in turn until one of them connects successfully or all addresses fail. There are a few differences from edgedb-python/js:

• deprecated features were omitted in this implementation.
• js always uses credentials file user & port, py uses credentials file user & port only if no user or port arguments were passed to connect(). This implementation follows edgedb-python.
• the exposed connect API is:

func Connect(ctx context.Context, opts Options) (*Pool, error)
func ConnectDSN(ctx context.Context, dsn string, opts Options) (*Pool, error)

func ConnectOne(ctx context.Context, opts Options) (*Conn, error)
func ConnectOneDSN(ctx context.Context, dsn string, opts Options) (*Conn, error)

I'm not sure I love this connect API. I'm not sure the DSN variants are necessary. Also, I almost want to change the whole thing to:

func DSN(dsn string) (Options, err)
func Instance(name string) (Options, err) // or maybe call this one CredentialsFile :thinking: 

func Connect(ctx context.Context, opts Options) (*Pool, error)
func ConnectOne(ctx context.Context, opts Options) (*Conn, error)

[fmoor]

pgx has several functions that all return the equivalent to Options then connect just takes an Options. This pattern seems common in go. However, this change would leave a lot of work up to the user if they wanted to merge a dsn or credential file with their own defaults the way that the current api allows or a builder API would need to be added to Options. I don't have a strong sense of which would be better 🤔

[tailhook]

Two big things:

1. I don't see why we need Options and connConfig. I would expose latter, with nice methods to create and modify the structure. Like this in Rust: https://github.com/edgedb/edgedb-rust/blob/master/edgedb-client/src/builder.rs#L90-L206 Because Options is a bit confusing and not forward-compatible

2. I also have doubts with errors. Currently you use a bunch of singletons as errors. This is a known pattern in Go, but the problem is that not all of the errors could be singletons. So users have to learn which are constants and use errors.Is with them and which are types and use errors.As with them. And also can't switch from singleton to type if some error has two different messages now. Is this what is considered normal for gophers?

[tailhook]

Not sure I understand structure here. Is this "Hosts Ports" or parallel arrays where first host in Hosts corresponds to first port in ports?

What are the downsides of this being an array of pairs?

[fmoor]

This was an attempt to follow the js driver behavior. The builder pattern would be much a nicer API in go it just didn't seem like that design was finalized. I'm just trying to get off the ground with this PR.

[tailhook]

I see an issue with defaults here. E.g. default user and database are edgedb, default port is 5656.

The problem with applying these defaults later is that, config like this:

{ database: "" }

Would be treated differently in no an non go. In all other languages it's the error (database name is empty). But because golang doesn't distinguish between empty string and non-existing string, it will be equivalent to database: "edgedb".

[tailhook]

Eventually, we want to hide these as private fields and use builder/fluent interface. I.e. only expose methods I think. Not sure if this has to be the same PR.

[fmoor]

1. I don't see why we need `Options` and `connConfig`. I would expose latter, with nice methods to create and modify the structure. Like this in Rust: https://github.com/edgedb/edgedb-rust/blob/master/edgedb-client/src/builder.rs#L90-L206 Because `Options` is a bit confusing and not forward-compatible

It was not clear to me that the builder approach was the accepted and final future design, so I opted to follow the current py & js implementations. If the builder approach is finalized I'm happy to change this PR. If it is not finalized yet, I'd prefer to merge this and rework it when it is finalized.

2. I also have doubts with errors. Currently you use a bunch of singletons as errors. This is a known pattern in Go, but the problem is that not all of the errors could be singletons. So users have to learn which are constants and use `errors.Is` with them and which are types and use `errors.As` with them. And also can't switch from singleton to type if some error has two different messages now. Is this what is considered normal for gophers?

This PR is not intended to provide a coherent error API. There is an issue for that here #24

[1st1]

This PR is not intended to provide a coherent error API. There is an issue for that here #24

Yeah, I'd merge this as is unless there are other objections and continue iterating on the API in follow up PRs.