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

PostgreSQL support broken since January 10, 2018 (release of pg 1.0.0) #368

Closed
yemartin opened this issue Feb 7, 2018 · 5 comments
Closed

Comments

@yemartin
Copy link

yemartin commented Feb 7, 2018

The release of version 1.0.0 of the pg gem on January 10, 2018 has broken PostgreSQL support. Steps to reproduce: create a new Custom application with PostgreSQL support. It shows the following error:

      option  Okay to drop all existing databases named epark-affiliates? 'No' will abort immediately! (y/n) y
         run    bundle exec rake db:drop from "."
rake aborted!
Gem::LoadError: Specified 'postgresql' for database adapter, but the gem is not loaded. Add `gem 'pg'` to your Gemfile (and ensure its version is at the minimum required by ActiveRecord).

This is a knows issue that will be addressed in Rails 5.1.5, but even when fixed in Rails, it will leave rails_apps_composer with broken PostgreSQL support for any earlier Rails version...

As specified on the Rails pull request that fixes the issue, Rails versions prior to 5.1.5 (and to 5.0.?? must use:

gem "pg", "~> 0.18"

So maybe rails_apps_composer should look at the Rails version, and create the pg Gemfile line accordingly?

@DanielKehoe
Copy link
Member

Yes, thanks for bringing it to my attention. I'll address it as soon.

@DanielKehoe
Copy link
Member

DanielKehoe commented Feb 9, 2018

@yemartin can you improve on my conditional logic, please?

if prefer :database, 'postgresql'
  if Rails::VERSION::MAJOR < 5
    add_gem 'pg', '~> 0.18'
  else
    if Rails::VERSION::MAJOR == 5 && Rails::VERSION::MINOR <= 1 && Rails::VERSION::MINOR <= 4
      add_gem 'pg', '~> 0.18'
    else
      add_gem 'pg'
    end
  end
end

@DanielKehoe
Copy link
Member

Fixed in rails_apps_composer version 3.1.31.

@DanielKehoe
Copy link
Member

@yemartin I've released the new rails_apps_composer version 3.1.31 and updated Rails Composer. Could you please test and reopen this issue if it's not resolved.

@yemartin
Copy link
Author

yemartin commented Feb 9, 2018

@DanielKehoe Thank you for the great response time!

I see you already released the fix, but in case you are still interested in improving the conditional logic, how about using Gem::Version to do the semantic version comparisons? It makes the code cleaner, easier to grok, and less error-prone. If we only care about the fix in 5.1.5, the code becomes simply:

# We may want to extract this somewhere to the top to make it reusable:
RAILS_VERSION = Gem::Version.new(Rails::VERSION::STRING)

if prefer :database, 'postgresql'
  if RAILS_VERSION < Gem::Version.new("5.1.5")
    add_gem 'pg', '~> 0.18'
  else
    add_gem 'pg'
  end
end

Now if we want to take into account that the fix was also backported to both Rails 5.0.7 and 5.1.5, it is a bit more complex, but still not too bad:

# We may want to extract this somewhere to the top to make it reusable:
RAILS_VERSION = Gem::Version.new(Rails::VERSION::STRING)

if prefer :database, 'postgresql'
  if RAILS_VERSION < Gem::Version.new("5.0.7") || \
    (RAILS_VERSION >= Gem::Version.new("5.1") && RAILS_VERSION < Gem::Version.new("5.1.5"))
    add_gem 'pg', '~> 0.18'
  else
    add_gem 'pg'
  end
end

I tested it locally and it seems to work. But now, I see the rest of the code does not use Gem::Version... Is there is a good reason against it?

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

No branches or pull requests

2 participants