Configuring with standard database options

[janko]

Currently the database is configured via the top-level :dbname, :dbadapter, :username, :password etc. options. What do you think that instead of that there is a single :database option which would allow configuring the database with the standard Sequel.connect options?

SequelTools.base_config(
  database: {
    adapter: "postgres",
    database: "mydb",
    # ...
  }
)
# or
SequelTools.base_config(
  database: "postgres:///mydb"
)

I think that would make it more friendly, because the users wouldn't need to remember how these options are named in SequelTools. And the same option could also be re-used for passing a connection string instead of a hash of options should the user prefer it (e.g. it might be convenient when the user has a $DATABASE_URL environment variable, e.g. when using Heroku).

[rosenfeld]

I'm not against grouping that information in the config like you suggesting, but I'm not sure it's a good idea to accept the uri itself for a few reasons. Long ago I noticed that with JRuby Sequel was failing to connect for some reason if I passed the hash to Sequel.connect, so I would build the URI from the params and pass to Sequel.connect instead and it would always work. Unfortunately I lost the context and I can no longer confirm this is the case, but this is just part of the reason why I think the configuration should always enforce those options separately rather than accepting an URI.

Some of the actions require access to those values separately. For example, take a look at the latest "shell" action added to SequelTools. It allows one to specify which script to run when the adapter is not supported or just in case someone would prefer to launch a separate CLI. The script will get the database name, port, host, username and password through environment variables, separately. If we accepted an URI, we'd have to implement parsing of it to extract those data from the URI.

So, if you'd like to submit a PR changing some options to the :database key, I'm all for it, I'm just not convinced about allowing an URI to be passed instead. What do you think?

[rosenfeld]

For the case where $DATABASE_URL is defined, such as when using Heroku, we might provide some URI decoding utility to try to make things easier for them, and see how it works. For example:

SequelTools.base_config(
  database: SequelTools.decode_connection_uri("postgres:///mydb")
)

Would that sound good to you?

[janko]

So, if you'd like to submit a PR changing some options to the :database key, I'm all for it, I'm just not convinced about allowing an URI to be passed instead.

What you said makes sense, it would be extra effort to parse the URI, so I'm ok with not supporting it. My main wish was to support the Hash style via the :database option.

For the case where $DATABASE_URL is defined, such as when using Heroku, we might provide some URI decoding utility to try to make things easier for them, and see how it works.

If we'd already be providing a SequelTools.decode_connection_uri, I think it would be convenient if it was automatically called when :database is a string.

I think it would make sense to accept a database URI only when the database already exists, because then we could just let Sequel do the parsing for us:

Sequel.connect("postgres:///mydb") { |db| db.opts } #=>
# {
#   :user=>nil,
#   :password=>nil,
#   :port=>nil,
#   :host=>nil,
#   :database=>"mydb",
#   :orig_opts=>{},
#   :uri=>"postgres:///mydb",
#   :adapter=>"postgres",
#   :adapter_class=>Sequel::Postgres::Database,
#   :single_threaded=>false
# }

[rosenfeld]

Like you said, Sequel doesn't provide a public method itself to parse a supported URI, which means it's hard to properly support this decoding for some edge cases, such as JDBC and so on. Just take a quick inspect at Database#connect's implementation. That's why I'd prefer to avoid trying to provide such a #decode_connection_uri in the first place. The URI seems to be very specific, while host/port/user/password/dbname are very well defined for basically any database. I'm also not much a fan of accepting multiple types in the same method. It usually complicates things in many levels, including the effort of those reading the code and trying to guess what it does depending on how the param is provided.

So, maybe, for convenience, we could define a very strict set of URI that we are able to parse with confidence, just for convenience sake. Such URI would look like "adapter://[user[:password]@][host]/dbname[:port]", while an empty host would mean localhost. Any other URI supported by Sequel itself would not be supported by SequelTools. It would be just to make writing something like this simpler:

# instead of:

database: {
  adapter: 'postgres',
  host: 'localhost',
  port: 5433,
  username: 'myuser',
  name: 'dbname',
  password: 'secret',
}

# we'd use instead:

database: SequelTools.decode_db_uri('postgres://myuser:secret@localhost:5433/dbname')

# or, for default adapter options means (environment variables / `~/.pgpass` / whatever):

database: SequelTools.decode_db_uri('postgres:///dbname')

[janko]

Ok, that definitely makes sense. I was thinking that the :database argument would just accept whatever Sequel.connect accepts, but that wouldn't make sense because we cannot and don't always want to Sequel.connect to the provided database (db:shell, db:create, db:drop).

I think that a SequelTools.decode_db_uri won't be necessary, and I think it might be surprising that in some cases the URI would be interpreted differently than how Sequel.connect would interpret it. In case the user has a database URL to an already created database, we could suggest them to use

database: Sequel.connect(database_url, &:opts)

[rosenfeld]

I was thinking more about providing a shorter way to provide those basic params, since that URI spec is a subset of the URIs supported by Sequel itself and people are already used to understand its meaning. Sequel also allows providing params using queries, such as "?username=me&password=secret", for example, but we wouldn't support that for simplicity sake. It would be just a shorter and simple alternative to provide common database params for those already used to that sort of URI, which is a subset of Sequel's supported URI by the way.

[rosenfeld]

Also, more work is needed to support JRuby. For that we need to connect using the JDBC URI, which is somewhat different from the one it's currently built for passing to Sequel. Instead of providing "user:password@" it's probably better to pass "?user=&password=" since it's supported both by Sequel and by the JDBC URI. Sequel will always use "jdbc:JDBC_URI" when connecting to JDBC, so the URI must be built for JRuby anyway. My idea is to detect JRuby is running and prepend "jdbc:" to the URI upon generation and replace "user:password@" with "?user=&password=" to also support JRuby. I need some more time to give it a try, but most likely I won't have time to take a look at this today as I'm leaving in a few minutes, but I'll try to add JRuby tests tomorrow.

[janko]

@rosenfeld What is the advantage of parsing the URI ourselves compared to having users call Sequel.connect which lets Sequel parse the URI for us?

database: Sequel.connect(database_url, &:opts)

Of course, Sequel.connect can only be used when the database already exists, but that's my main use case for using a URI anyway (on Heroku or AWS RDS the database is already created for you).

That wouldn't require SequelTools to add any additional functionality regarding database URIs, because the parsing would be done on the user-side, SequelTools would just accept the parsed Hash which we already agreed we want it to accept.

[rosenfeld]

The advantage would be to provide a shorter alternative to specify the parameters. Just a matter of saving some time/bytes setting up the Rakefile ;)

[janko]

The advantage would be to provide a shorter alternative to specify the parameters. Just a matter of saving some time/bytes setting up the Rakefile ;)

That sounds like the advantage of allowing a database URI as an alternative to a hash of options. What I'm asking is what is the advantage of using

database: SequelTools.decode_db_uri('postgres:///dbname')

which needs to be implemented and will not work correctly with some database URIs (e.g. JDBC), compared to

database: Sequel.connect('postgres:///dbname', &:opts)

which doesn't need to be implemented and will work correctly for any database URI?

[rosenfeld]

Those are separate things. For configuring SequelTools, whether JDBC is going to be used or not is irrelevant. The URI would be just a matter of providing a simpler way to specify the following params: host, port, adapter, username, password and database name.

Then it's SequelTools the responsible for translating that accordingly. For example, when using JDBC, SequelTools would generate some JDBC connection string like "jdbc:postgresql://localhost/mydb?user=myuser&password=secret" if the URI passed to SequelTools was 'postgres://myuser:secret@localhost/mydb'.

The postgresql adapter would take care of using "postgresql" instead of "postgres" when passing to the JDBC connection string when it detects that RUBY_PLATFORM is 'java', as part of the process of building the connection string to pass to Sequel.

For configuration purpose, we only have to support the basic options: user, password, host, port, database name and adapter. We can easily support that with a well defined URI schema. Then it's up to SequelTools to translate that to something Sequel would understand.

[janko]

For those curious, this is how I set up SequelTools from a database URL:

db_uri  = URI.parse(database_url)
db_opts = Sequel::Database.send(:uri_to_options, db_uri)

base_config = SequelTools.base_config(
  project_root: __dir__,
  dbadapter:    db_uri.scheme,
  dbname:       db_opts[:database],
  dbhost:       db_opts[:host],
  dbport:       db_opts[:port],
  username:     db_opts[:user] || "",
  password:     db_opts[:password] || "",
  ...
)