Skip to content
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.

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

Add support of ENV['DATABASE_URL'] #54

Closed

Conversation

donbobka
Copy link

@donbobka donbobka commented Feb 3, 2015

My try to add support of environment variable DATABASE_URL. In my case I set DATABASE_URL with gem figaro.

The main change is adding servedside rake task capistrano_db_tasks:config that returns remote database configuration.

Side-changes:

  • Add .gitignore, add .editorconfig
  • Move all capistrano specific files to capistrano directory.

P.S.: Changes are not backward compatible with previous versions.

@numbata
Copy link
Collaborator

numbata commented Apr 27, 2015

Thanks for pull-request, but you did a lot of changes. I'm okay with .gitignore, but not with .editorconfig. I think it should be in your ~/.gitignore_global, because it is your personal setting + someone use vim, someone rubymine, some one prefer emacs.
It's okay about adding DATABASE_URL support, but not with restructurese in one PR. Can you split it onto two PR (one for DATABASE_URL support and one with restructure).

@donbobka
Copy link
Author

About .editorconfig: it's not about personal choice, it's about common code style for a project. For example, look at ruby repo: https://github.com/ruby/ruby/blob/trunk/.editorconfig
There are a lot of plugins(http://editorconfig.org/#download) for a different editors to support .editorconfig file.

About PR spliting: I'll try to do it

@numbata
Copy link
Collaborator

numbata commented Apr 27, 2015

@donbobka thanks for the explanation. You're right.

@fabn
Copy link

fabn commented Jul 22, 2015

Very nice PR. DATABASE_URL support is a must for production environments. Any chance to see this merged?

@donbobka Can I point my Gemfile to your fork? Is it stable or it will be rebased/changed to get this merged?

@fabn
Copy link

fabn commented Jul 29, 2015

I tried this PR using your branch and it works with DATABASE_URL however I had to add the gem in production group to make it working. Why don't you implement the fetch config in a cap task? In this way this gem can remain in development group with no need to have it on server. Something like

  on roles(:app) do
    within release_path do
      execute(:bundle, 'exec rails runner "ActiveRecord::Base.configurations[Rails.env].to_yaml" > tmp/config.yml')
      db_config = capture('cat tmp/config.yml')
      execute 'rm tmp/config.yml'
    end
  end

numbata added a commit to numbata/capistrano-db-tasks that referenced this pull request Sep 9, 2015
numbata added a commit to numbata/capistrano-db-tasks that referenced this pull request Sep 17, 2015
numbata added a commit to numbata/capistrano-db-tasks that referenced this pull request Oct 7, 2015
numbata added a commit to numbata/capistrano-db-tasks that referenced this pull request Oct 14, 2015
artempartos pushed a commit to artempartos/capistrano-db-tasks that referenced this pull request Aug 17, 2016
artempartos pushed a commit to artempartos/capistrano-db-tasks that referenced this pull request Aug 17, 2016
artempartos pushed a commit to artempartos/capistrano-db-tasks that referenced this pull request Aug 17, 2016
artempartos pushed a commit to artempartos/capistrano-db-tasks that referenced this pull request Aug 17, 2016
numbata added a commit to numbata/capistrano-db-tasks that referenced this pull request Aug 18, 2016
numbata added a commit to numbata/capistrano-db-tasks that referenced this pull request Sep 20, 2016
numbata added a commit to numbata/capistrano-db-tasks that referenced this pull request Sep 20, 2016
numbata added a commit that referenced this pull request Sep 20, 2016
numbata added a commit to numbata/capistrano-db-tasks that referenced this pull request Sep 26, 2016
@numbata
Copy link
Collaborator

numbata commented Oct 7, 2016

So, i close this PR now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants