Skip to content

Simplify setup #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
janko wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Simplify setup #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f6d797d
Default `:project_root` to `Dir.pwd`
janko Oct 19, 2022
775341a
Remove the need for calling `SequelTools.base_config`
janko Oct 19, 2022
059f9b4
Support `:db_url` for DB config options
janko Oct 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,24 @@ And then execute:

## Usage

The simplest way to get started:

```rb
require 'sequel_tools'

namespace :db do
SequelTools.inject_rake_tasks Hash[db_url: 'postgres://username:password@localhost/dbname'], self
end
```

Here's a sample Rakefile supporting migrate actions:

```ruby
require 'bundler/setup'
require 'sequel_tools'

base_config = SequelTools.base_config(
project_root: File.expand_path(__dir__),
project_root: __dir__,
dbadapter: 'postgres',
dbname: 'mydb',
username: 'myuser',
Expand Down
27 changes: 22 additions & 5 deletions lib/sequel_tools.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
# frozen-string-literal: true

require 'sequel_tools/version'
require 'sequel/core'
require 'uri'

module SequelTools
DEFAULT_CONFIG = {
project_root: nil,
project_root: Dir.pwd,
pg_dump: 'pg_dump', # command used to run pg_dump
psql: 'psql', # command used to run psql
maintenancedb: :default, # DB to connect to for creating/dropping databases
migrations_location: 'db/migrations',
schema_location: 'db/migrations/schema.sql',
seeds_location: 'db/seeds.rb',
db_url: nil,
dbname: nil,
dbhost: 'localhost',
dbadapter: 'postgres',
Expand All @@ -24,13 +27,27 @@ module SequelTools
extra_tables_in_dump: nil,
} # unfrozen on purpose so that one might want to update the defaults

REQUIRED_KEYS = [ :project_root, :dbadapter, :dbname, :username ]
class MissingConfigError < StandardError; end
def self.base_config(extra_config = {})
config = DEFAULT_CONFIG.merge extra_config
REQUIRED_KEYS.each do |key|
raise MissingConfigError, "Expected value for #{key} config is missing" if config[key].nil?
unless config[:db_url] || (config[:dbadapter && config[:dbname]])
raise MissingConfigError, "Must provide either :db_url or :dbadapter and :dbname config options"
end

if config[:db_url]
db = Sequel.connect(config[:db_url], test: false, keep_reference: false)
if RUBY_PLATFORM == 'java'
uri = URI.parse(config[:db_url][/\Ajdbc:(.+)/, 1])
config[:dbadapter] = uri.scheme
config[:dbname] = uri.path[/\/(.+)/, 1]
else
config[:dbadapter] = db.opts[:adapter]
config[:dbname] = db.opts[:database]
end
config[:username] = db.opts[:user]
config[:password] = db.opts[:password]
end

[:migrations_location, :schema_location, :seeds_location].each do |k|
config[k] = File.expand_path config[k], config[:project_root]
end
Expand All @@ -40,7 +57,7 @@ def self.base_config(extra_config = {})
def self.inject_rake_tasks(config = {}, rake_context)
require_relative 'sequel_tools/actions_manager'
require_relative 'sequel_tools/all_actions'
actions_manager = ActionsManager.new config
actions_manager = ActionsManager.new base_config(config)
Copy link
Owner

@rosenfeld rosenfeld Oct 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

@janko janko Oct 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 🙂

actions_manager.load_all
actions_manager.export_as_rake_tasks rake_context
end
Expand Down