segmentio / go-athena

Golang database/sql driver for AWS Athena
MIT License
117 stars 64 forks source link

Allow to pass region as parameter in connection string #7

Closed funny-falcon closed 6 years ago

funny-falcon commented 6 years ago

It is a bit cumbersome to specify region when it is not specified in environment variable, and there is no reason to not allow it to be passed in connection string, imho.

tejasmanohar commented 6 years ago

@funny-falcon Untested, but I think the below code does what you want. Should it be in the README?

db, err := athena.Open(Config{
  Session: session.Must(session.NewSession(&aws.Config{
    Region: aws.String("YOUR AWS REGION"),
  }))
})

// db is a sql.DB

FWIW, I'm ok supporting more options in the connection string, but I think we should support the three most common options to avoid more PRs :) 1) aws_access_key_id 2) aws_secret_access_key 3) aws_region

If you decide to update the PR, please also update the godoc comment on Driver.Open().

tejasmanohar commented 6 years ago

If you're feeling generous (optional), I think a quick section in the README explaining the 3 different ways (env, creds in connection string, athena.Open()) to authenticate with AWS would help, too 😄

funny-falcon commented 6 years ago

access_key_id and secret_access_key are "dangerous". And they could be set at EC2 level. region is safe, and could not be set at EC2 level (amazon, why? the most stupid decision, I think) May I not add access_key and secret_access_key ?

Untested, but I think the below code does what you want. Should it be in the README?

Certainly

tejasmanohar commented 6 years ago

SGTM. We don't use raw AWS keys either. Merging.

tejasmanohar commented 6 years ago

And as always, thanks for contributing