Simplify setup

[janko]

This pull request is a compilation of changes that aim to reduce friction when configuring rake tasks.

I found out that, when rake is called from a subdirectory, Dir.pwd will still point to the directory containing the rakefile. This is because Rake internally uses Dir.chdir, which updates Dir.pwd. This means we don't need to require :project_root anymore, we can default to Dir.pwd.

Next, it would be convenient if there was no need to call SequelTools.base_config, for cases when development & test databases don't share common configuration, or there is only one set of rake tasks. Currently, Sequel.base_config requires :dbname and :username to be passed in, but those will almost always be different between databases. We achieve this by calling SequelTools.base_config from SequelTools.inject_rake_tasks.

Finally, following up to #2, we add a :db_url option, which allows specifying a database URL instead of database options. It's common for applications to have database URLs in their application config, so this should be most convenient. We also don't use any private Sequel APIs. I chose not to support passing Sequel::Database objects, because for db:drop to work there shouldn't be any open connections, so I didn't want people to be able to fall into that trap.

[janko]

I didn't manage to setup the databases locally for testing, so I didn't write any tests at the moment. I mostly wanted to check if you would accept these changes. My goal is to make SequelTools as convenient as possible, as I currently still feel friction when setting it up in a new project.

[rosenfeld]

Hi janko, great to interact with you again after a long while. I'll take a look at these changes carefully tonight.

I'm mostly worried about the URL part. I think I tried to use the URL at first for setting the connection but stumbled upon some issues with JRuby. It's easier to build the URL from the connection settings rather than the other way around. For example, for PostgreSQL, with MRI we use the postgres:// URL while in JRuby it could be based on jdbc. There's a while since I created this and I have been using this gem as is for a long while, so I need some time to remember all those caveats. I'm in a hurry today to finish some tasks, but I promise I'll take a look at it carefully tonight and get back to you.

Thanks for the PR.

[janko]

Yeah, it's nice to interact with you too again 🙂. You're right, my solution doesn't work with JRuby, db.opts[:adapter] returns nil. I could probably get away with using #adapter_scheme and #database_type methods:

DB = Sequel.connect("jdbc:postgresql:///janko")
DB.adapter_scheme #=> :jdbc
DB.database_type #=> :postgres

Though database_type won't necessarily match the sub-adapter name, AFAIK. I will do some more testing.

[janko]

Added JRuby support, which I tested with migrations and creating/dropping the database, and it seems to work 🤞🏻

[rosenfeld]

Hi @janko. The main purpose of calling base_config here is not really about sharing configs for different environments. I can see that this name is misleading and could be improved. But base_config does 3 things:

1 - set the default options when not specified;
2 - ensure all config params are actually valid to avoid people spending a lot of time trying to understand why it is not working as expected because they mistyped some option.
3 - it expands the relative paths to migrations_location, schema_location and seeds_location.

I don't really understand the reasoning behind this commit. Why removing base_config would help you setting up your projects? I mean, why troubles calling base_config cause to your projects?

[janko]

The main benefit is smaller API surface to remember. When I first saw base_config, I thought it created something that resembled a Hash but isn't a Hash, which increased complexity in my mind. All of the things you describe can also happen in inject_rake_tasks, and it's how most libraries work (e.g. Shrine's storage classes set default values and assert on required arguments directly in #initialize).

Consider the following example:

base_config = SequelTools.base_config(dbadapter: "postgresql", dbname: "myapp_development", username: "")

namespace "db" do
  SequelTools.inject_rake_tasks base_config.merge(log_level: :info, sql_log_level: :info), self
end

namespace "db:test" do
  SequelTools.inject_rake_tasks base_config.merge(dbname: "myapp_test"), self
end

Here I needed to set :dbname in base config, even though it's going to differ from test database, which I don't find intuitive. Not needing to use base_config allows the user to share only the common config if they wanted to, and you still get all those checks & conversion of arguments:

base_config = { dbadapter: "postgresql" }

namespace "db" do
  SequelTools.inject_rake_tasks base_config.merge(dbname: "myapp_development", log_level: :info, sql_log_level: :info), self
end

namespace "db:test" do
  SequelTools.inject_rake_tasks base_config.merge(dbname: "myapp_test"), self
end

And in case of single set of rake tasks, you get to avoid defining a local variable, and have just a single call. As a Rubyist I like this; a lot of Ruby's standard library helps avoid defining local variables. So, while it doesn't cause any troubles, it does gives me more freedom 🙂