Allow to pass a connection to the `dbconsole` command #29358

robin850 · 2017-06-05T21:29:24Z

Hello,

Since 0a4f600, it's possible to specify a 3-level database configuration to gather connections by environment.

The dbconsole command will try to look for a database configuration which points to the current environment but with such flavour, the environment key is flushed out so let's add the ability to specify the connection and pick primary by default to be consistent with Active Record.

Have a nice day !

y-yagi · 2017-06-05T22:47:26Z

railties/lib/rails/commands/dbconsole/dbconsole_command.rb

@@ -99,6 +99,10 @@ def environment
      Rails.respond_to?(:env) ? Rails.env : Rails::Command.environment
    end

+    def connection
+      @options[:connection] || "primary"


Can we use thor's default value?

class_option :connection, aliases: "-c", type: :string, desc: "Specifies the connection to use.", default: "primary"

Yes, totally fair point and thus, this will be printed running --help, thanks !

Actually I added it back because Rails::DBConsole is public API so people may be using it directly, without Thor. 😊

jeremy

Yes please!

jeremy · 2017-06-08T00:34:53Z

railties/lib/rails/commands/dbconsole/dbconsole_command.rb

@@ -87,10 +87,10 @@ def start

    def config
      @config ||= begin
-        if configurations[environment].blank?
+        if configurations[environment].blank? && configurations[connection].blank?
          raise ActiveRecord::AdapterNotSpecified, "'#{environment}' database is not configured. Available configuration: #{configurations.inspect}"


This error will be misleading if the connection was provided.

jeremy · 2017-06-08T00:38:36Z

railties/lib/rails/commands/dbconsole/dbconsole_command.rb

          raise ActiveRecord::AdapterNotSpecified, "'#{environment}' database is not configured. Available configuration: #{configurations.inspect}"
        else
-          configurations[environment]
+          configurations[environment] || configurations[connection]


If configurations[environment] is {} and configurations[connection] is present, we end up returning the blank env config here instead of the conn config.

jeremy · 2017-06-08T00:39:00Z

railties/lib/rails/commands/dbconsole/dbconsole_command.rb

+    def connection
+      @options["connection"] || "primary"
+    end
+


Nice case for Hash#fetch

jeremy · 2017-06-08T00:42:06Z

railties/lib/rails/commands/dbconsole/dbconsole_command.rb

@@ -145,6 +149,9 @@ class DbconsoleCommand < Base # :nodoc:
      class_option :environment, aliases: "-e", type: :string,
        desc: "Specifies the environment to run this console under (test/development/production)."

+      class_option :connection, aliases: "-c", type: :string,
+        desc: "Specifies the connection to use.", default: "primary"
+
      def perform
        extract_environment_option_from_argument


I'd expect to be able to provide the conn as a plain arg. Example scenarios/possibilities:

rails dbconsole production replica rails dbconsole production:replica RAILS_ENV=production rails dbconsole replica rails dbconsole reporting-replica

Think we should deprecate the positional environment arg then and have:

rails dbconsole replica -e production

jeremy · 2017-06-08T00:43:03Z

railties/CHANGELOG.md

+    command when using a 3-level database configuration.
+
+    *Robin Dupret*
+


Let's show an example of this invocation style, too 👍

robin850 · 2017-06-08T23:02:17Z

Jeremy, there is now a presence check in case configurations[environment] is {} but I don't see how that could happen because this would've been catch by blank? above. Also, with the new configuration system, Active Record is either returning a) a configuration with the environment's as a key (2-level db config) or b) a configuration without any occurrence of the current environment but rather a key for each possible connection (3-level db config). We can keep it but I'm not sure this is useful.

Kasper, passing the environment as a regular argument is deprecated as you suggested ; this is certainly going to be easier for us to handle only the connection as a regular argument. Such flavor is not deprecated for the console command as I don't know whether we are going to open all available connections by default (c.f. #29360) or let people open them manually (thus, we would have to do that for console as well).

Finally, there's a tiny fix attached with this pull request because if we are going to deprecate passing the environment as a regular argument, people may encounter issues as the -e notation didn't expand shortcuts (like dev for development).

Thanks for the feedback guys ! :-)

kaspth · 2017-07-15T19:00:12Z

Can you rebase? The changelog conflicts.

I think we should deprecate the environment positional command argument for console as well for consistency.

Since 0a4f600, it's possible to specify a 3-level database configuration to gather connections by environment. The `dbconsole` command will try to look for a database configuration which points to the current environment but with such flavour, the environment key is flushed out so let's add the ability to specify the connection and pick `primary` by default to be consistent with Active Record.

People should rather rely on the `-e` or `--environment` options to specify in which environment they want to work. This will allow us to specify the connection to pick as a regular argument in the future.

Running the `console` and `dbconsole` commands with a regular argument as the environment's name automatically expand it to match an existing environment (e.g. dev for development). This feature wasn't available using the `--environment` (a.k.a `-e`) option.

robin850 · 2017-07-16T13:58:03Z

@kaspth : Rebased and deprecated the environment argument for the console command.

kaspth · 2017-07-16T19:02:09Z

railties/lib/rails/command/environment_argument.rb

      end

      private
        def extract_environment_option_from_argument
          if environment
            self.options = options.merge(environment: acceptable_environment(environment))
-          elsif !options[:environment]
+
+            ActiveSupport::Deprecation.warn "Passing the environment's name as a " \


Much cleaner to have this straight in here 👏

I mistype `rails server production` instead of `rails server -e production` expecting to lunch a server in the production environment all the time. However, the signature of `rails server --help` is: ``` Usage: rails server [puma, thin etc] [options] ``` This means that the `production` argument is being interpreted as a Rack server handler like Puma, Thin or Unicorn. Should we argue for the `rails server production`? I'm not sure of the reasons, but the `rails console production` behavior was deprecated in: rails#29358, so parity with the existing `rails console production` usage may not hold anymore. In any case, this PR introduces an explicit option for the Rack servers configuration. The option is called `--using` (or `-u` for short) to avoid the `rails server --server` tantrum. The new interface of `rails server` is: ``` Usage: rails server [using] [options] Options: -p, [--port=port] # Runs Rails on the specified port - defaults to 3000. -b, [--binding=IP] # Binds Rails to the specified IP - defaults to 'localhost' in development and '0.0.0.0' in other environments'. -c, [--config=file] # Uses a custom rackup configuration. # Default: config.ru -d, [--daemon], [--no-daemon] # Runs server as a Daemon. -e, [--environment=name] # Specifies the environment to run this server under (development/test/production). -u, [--using=name] # Specifies the Rack server used to run the application (thin/puma/webrick). -P, [--pid=PID] # Specifies the PID file. # Default: tmp/pids/server.pid -C, [--dev-caching], [--no-dev-caching] # Specifies whether to perform caching in development. [--early-hints], [--no-early-hints] # Enables HTTP/2 early hints. ``` As a bonus, if you mistype the server to use, you'll get an auto-correction message: ``` $ rails s tin Could not find handler "tin". Maybe you meant "thin" or "cgi"? Run `rails server --help` for more options. ```

This commit resolves deprecation warnings like below: >DEPRECATION WARNING: Passing the environment's name as a regular argument is deprecated and will be removed in the next Rails version. Please, use the -e option instead. cf. rails/rails#29358

robin850 added the railties label Jun 5, 2017

y-yagi reviewed Jun 5, 2017

View reviewed changes

jeremy reviewed Jun 8, 2017

View reviewed changes

robin850 added 3 commits July 16, 2017 14:39

Deprecate environment as an argument for dbconsole and console

48b2499

People should rather rely on the `-e` or `--environment` options to specify in which environment they want to work. This will allow us to specify the connection to pick as a regular argument in the future.

kaspth reviewed Jul 16, 2017

View reviewed changes

kaspth merged commit c74820d into rails:master Jul 16, 2017

robin850 deleted the dbconsole-connection branch July 16, 2017 20:59

gsamokovarov mentioned this pull request Feb 19, 2018

Introduce explicit rails server handler option #32058

Merged

abicky mentioned this pull request Jan 29, 2019

Resolve deprecation warnings on inf-ruby-console-rails nonsequitur/inf-ruby#119

Merged

eugeneius mentioned this pull request May 24, 2021

Support using replicas when using rails dbconsole #42285

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Allow to pass a connection to the `dbconsole` command #29358

Allow to pass a connection to the `dbconsole` command #29358

Uh oh!

robin850 commented Jun 5, 2017

Uh oh!

y-yagi Jun 5, 2017

Uh oh!

robin850 Jun 5, 2017

Uh oh!

robin850 Jun 6, 2017

Uh oh!

jeremy left a comment

Uh oh!

jeremy Jun 8, 2017

Uh oh!

jeremy Jun 8, 2017

Uh oh!

jeremy Jun 8, 2017

Uh oh!

jeremy Jun 8, 2017

Uh oh!

kaspth Jun 8, 2017

Uh oh!

jeremy Jun 8, 2017

Uh oh!

robin850 commented Jun 8, 2017 •

edited

Loading

Uh oh!

kaspth commented Jul 15, 2017

Uh oh!

robin850 commented Jul 16, 2017

Uh oh!

kaspth Jul 16, 2017

Uh oh!

Uh oh!

		command when using a 3-level database configuration.

		Robin Dupret

Allow to pass a connection to the dbconsole command #29358

Allow to pass a connection to the dbconsole command #29358

Uh oh!

Conversation

robin850 commented Jun 5, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeremy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robin850 commented Jun 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaspth commented Jul 15, 2017

Uh oh!

robin850 commented Jul 16, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Allow to pass a connection to the `dbconsole` command #29358

Allow to pass a connection to the `dbconsole` command #29358

robin850 commented Jun 8, 2017 •

edited

Loading