Support for ERB database.yml ?

tdeo commented 3 years ago

Help us track down and resolve parity problems faster with this template.

What command did you execute?

development restore-from production

What did you expect to happen?

I expected the command to succeed

What actually happened?

It failed with the following message :

Traceback (most recent call last):
    11: from /usr/bin/development:9:in `<main>'
    10: from /usr/lib/ruby/vendor_ruby/parity/environment.rb:13:in `run'
     9: from /usr/lib/ruby/vendor_ruby/parity/environment.rb:24:in `run_command'
     8: from /usr/lib/ruby/vendor_ruby/parity/environment.rb:61:in `restore'
     7: from /usr/lib/ruby/vendor_ruby/parity/backup.rb:17:in `restore'
     6: from /usr/lib/ruby/vendor_ruby/parity/backup.rb:40:in `restore_to_development'
     5: from /usr/lib/ruby/vendor_ruby/parity/backup.rb:48:in `wipe_development_database'
     4: from /usr/lib/ruby/vendor_ruby/parity/backup.rb:108:in `development_db'
     3: from /usr/lib/ruby/2.7.0/psych.rb:277:in `load'
     2: from /usr/lib/ruby/2.7.0/psych.rb:390:in `parse'
     1: from /usr/lib/ruby/2.7.0/psych.rb:456:in `parse_stream'
/usr/lib/ruby/2.7.0/psych.rb:456:in `parse': (<unknown>): could not find expected ':' while scanning a simple key at line 33 column 1 (Psych::SyntaxError)

It seems that parity fails to load my config/database.yml file because I have ERB syntax in it, which is supported by rails even though the extension is not .yml.erb, here is a sample of it:

default: &default
  adapter: postgresql
  encoding: unicode
  url: <%= ENV['DATABASE_URL'] %>
  prepared_statements: false # required for connection pooling !
  pool: <%= [ENV['RAILS_MAX_THREADS'], ENV['SIDEKIQ_CONCURRENCY']].compact.map(&:to_i).max || 10 %>
  connect_timeout: 1
  checkout_timeout: 1

default_replica: &replica
  url: <%= ENV['DATABASE_REPLICA_URL'] %>
  replica: true

<%
def config(env, inherit_from: env)
  if Rails.configuration.has_replication
    <<~YAML
      #{env}:
        primary:
          <<: *#{inherit_from}
        replica:
          <<: *#{inherit_from}
          <<: *replica
    YAML
  else
    <<~YAML
      #{env}:
        <<: *#{inherit_from}
    YAML
  end
end
%>

development_default: &development
  <<: *default
  database: dev

<%= config('development') %>

test_default: &test
  <<: *default
  database: test

<%= config('test') %>

<%= config('production', inherit_from: 'default') %>

The file is loaded properly by rails, it seems like parity should load the file using ERB instead of plain yaml

Some information about your installation

What's your operating system?

$ uname -ar
Linux brewedbeer 5.4.0-48-generic #52-Ubuntu SMP Thu Sep 10 10:58:49 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux

What's the output of which development, which staging, which production? Parity has had multiple installation channels, and it's not uncommon for an old version to be somewhere else in your path.
```
$ which development 
/usr/bin/development
$ which staging
/usr/bin/staging
$ which production
/usr/bin/production
```
If installed via Homebrew, what does brew list parity output? N/A
If installed via Rubygems, what's the gem version? N/A

I installed parity via apt

geoffharcourt commented 3 years ago

Hi @tdeo, we removed ERB support to increase portability in https://github.com/thoughtbot/parity/commit/5704201d5bb7d8730e39e8f2767ae7964193a69d. PR #182 brings back some configuration support, but parsing ERB would require us to include an ERB gem and we've found that Travelling Ruby and Bundler don't play well together due a Bundler issue that will never be fixed.

I'd love to support configurability here, so if you have any ideas and can contribute a PR it would plug a major functionality gap for us.

tdeo commented 3 years ago

Actually, erb is part of the ruby stdlib (at least in c-ruby), and while it's true that basic interpolation in database.yml like the one that appears in a rails scaffold:

default:
  url: <%= ENV['DATABASE_URL'] %>

is compatible with plain-yaml parsing, any interpolation outside a text value will break yaml loading.

Is re-including erb from the stdlib out of the question, even without reintroducing bundler and gems ? I'm really not familiar with Travelling Ruby and unsure what's included

geoffharcourt commented 3 years ago

This is good news! If this is possible without adding any gems via Bundler (looks like that PR also removed git support, which might have been why I was wrong about ERB), then I don't see a reason why we can't do it.

Take a peek at #182, we'll want to be careful about making sure that solution and whatever happens here don't fall into conflict.

Thanks for figuring this out.

thoughtbot / parity