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

Remove Rails dependency (version bump 2.5.0) #127

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

nimmolo
Copy link

@nimmolo nimmolo commented May 31, 2022

This commit removes Rails as a dependency, requiring only the Rails component gems/classes necessary to run Rails DB. It is intended to make the gem more compatible with Rails apps that are similarly configured. Tests are passing for me.

  • Note that the require statements in bin/rails and test/config/application.rb are now more specific — they must specifically require the classes loaded by the gems, not just require 'rails'. The list is slightly different from the gem list, but this is a fairly standard setup that was easy to find examples on the web.

  • I moved the gemfiles list into rails_db.gemspec. I believe that with gems, the .gemspec takes precedence over Gemfile, and is redundant where the Gemfile lists gemspec. When the gems were listed in both places, Bundler was complaining that several gems were included twice.

  • I read in several places that for gem development, the Gemfile.lock should not be checked into version control. So I added it to .gitignore, but it doesn't seem to be ignored :) Anyway you can check the differences.

  • In test_helper.rb, to get the rails version, it's now checking the version of activerecord, which should always be the same as Rails anyway.

Rakefile Show resolved Hide resolved
@@ -15,24 +15,38 @@ Gem::Specification.new do |s|
s.files = Dir["{app,config,lib,test}/**/*", "rails_db.gemspec", "Gemfile", "Gemfile.lock", "MIT-LICENSE", "Rakefile", "README.rdoc", "bin/rails_db", "bin/railsdb", "bin/runsql"]
s.test_files = Dir["test/**/*"]

s.executables = ["railsdb", "rails_db", 'runsql']
s.executables = ['railsdb', 'rails_db', 'runsql']
Copy link
Owner

Choose a reason for hiding this comment

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

did you tried to run rails_db ? (I mean executables if works too?)

Copy link
Owner

Choose a reason for hiding this comment

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

and runsql

Copy link
Author

Choose a reason for hiding this comment

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

agh! You're right, the executables don't work. Only the GUI. Sorry i didn't catch that before.

standalone.rb is throwing an error when i try to run railsdb, it says that

/home/vagrant/.rvm/gems/ruby-3.0.4/gems/rack-2.2.4/lib/rack/handler/webrick.rb:26:in `run': 
wrong number of arguments (given 2, expected 1) (ArgumentError)

If I eliminate the options hash in line 46 of standalone.rb, it seems to launch but is not available at port :12345, it uses the WEBrick default port :8080. However, the Rack::Handler::WEBrick method run clearly should accept two arguments, app and options.

module Rack
  module Handler
    class WEBrick < ::WEBrick::HTTPServlet::AbstractServlet
      def self.run(app, **options)
        environment  = ENV['RACK_ENV'] || 'development'
        default_host = environment == 'development' ? 'localhost' : nil

        if !options[:BindAddress] || options[:Host]
          options[:BindAddress] = options.delete(:Host) || default_host
        end
        options[:Port] ||= 8080
        if options[:SSLEnable]
          require 'webrick/https'
        end

        @server = ::WEBrick::HTTPServer.new(options)
        @server.mount "/", Rack::Handler::WEBrick, app
        yield @server  if block_given?
        @server.start
      end

      def self.valid_options
        environment  = ENV['RACK_ENV'] || 'development'
        default_host = environment == 'development' ? 'localhost' : '0.0.0.0'

        {
          "Host=HOST" => "Hostname to listen on (default: #{default_host})",
          "Port=PORT" => "Port to listen on (default: 8080)",
        }
      end
...

If you have a chance, maybe you can help me debug why these two executables aren't launching.

@igorkasyanchuk
Copy link
Owner

interesting PR, thanks, need to think more about it.

Just to confirm - you have reduced the amount of dependencies, and not added any new?

@nimmolo
Copy link
Author

nimmolo commented Sep 2, 2022

@igorkasyanchuk That's right, no new dependencies.
But see answer to your question above, sorry.

@igorkasyanchuk
Copy link
Owner

@nimmolo could you please check this PR again, sorry long time, but maybe it still can be finished

@nimmolo
Copy link
Author

nimmolo commented May 12, 2023 via email

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.

2 participants