Add Target command to configure target databases #100

Closed
theory opened this Issue Jul 4, 2013 · 17 comments

Projects

None yet

4 participants

@theory
Owner
theory commented Jul 4, 2013

It would work sort of like Git remotes. Basically, it allows one to configure multiple targets for deployment. Currently this can only be done by command-line options (--db-name, --db-user, etc.) or configuration settings (core.pg.db_name, core.pg.user, etc.). The former is good for deploying to ad-hoc systems, but the latter locks you into a single target.

So allow multiple named database “targets” to be specified. These would be key/value pairs, where the key is a name (e.g., "dev", "qa", "prod", etc.) and the value is a URI with connection info. The Postgres URI format is a useful example to go with. We would use the engine name for the protocol section of the URI. Some examples for the different engines.

pg://foo:blah@localhost:5432/otherdb?sqitch_schema=meta
oracle://scott:tiger@db.example.com/orcl?sqitch_schema=woof
mysql://root@localhost/tryme?sqitch_db=har
sqlite:///var/db/flip.db?sqitch_db=/var/db/meta.db

In the configuration file, they might look like this:

[target]
   dev = pg://localhost/flipr_test
   qa = pg://qa.example.com/flipr?sqitch_schema=meta
   prod = pg://db.example.com/flipr

This usage specifies different environments.

The target command would insert these into the configuration file, so they could of course also be set by the config command. They are key-value pairs, of course. The keys should probably adhere to normal Sqitch object name rules. They can then be passed as arguments to the deploy, revert, rebase, checkout, etc. commands --- any that hit the database, and therefore need a database to query. One can also pass a raw URI to these commands, something not configured. Some examples using the above config:

sqitch checkout somebranch dev -y
sqitch rebase qa -y
sqitch deploy prod
sqitch revert pg://dev.example.com/blarg

There should probably be some core config option to specify a default target, the value of which could also be either a name or a URI.

Calling sqitch target with no params will list the databases. sqitch database -v will list the databases and their URIs.

@theory
Owner
theory commented Jul 4, 2013

Commands should throw an exception if the "protocol" is not the same as the value of --engine (or core.engine, of course).

@theory
Owner
theory commented Jul 4, 2013

And in fact, we might want to deprecate the core --db-* options once we have this.

@leklund
leklund commented Jul 18, 2013

+1
This feature will make life so much easier than the the multiple bash aliases I've been using.

@andrewalker

Since we're at it... would it be possible to interactively ask for password, when defining a target in the command line? I mean, both --db-name and --db-user are almost useless if they require the password to be blank. Something like "psql -W" or "mysql -p". As far as I can tell, the only way to define passwords currently would be in the config files (or use env vars). Am I missing something?

Anyway, git remote supports that :) Even delegating to keyrings, ssh-agent, etc.

@theory
Owner
theory commented Jul 18, 2013

Yeah, might be possible. The Pg engine can use it to set PGPASSWORD, while the MySQL and Oracle engines can pass it on the command-line (security FTW!).

Anyone want to take a stab at a patch?

@andrewalker

I might try it over the weekend!
I'd have to alter core, to ask for the password (given an argument?), and update the engines to set them like you said. Is that right? Any further tips?

@theory
Owner
theory commented Jul 20, 2013

I think you would just need to modify the default sub for the password (or db_pass) attribute in each engine subclass to fall back on Term::ReadPassword. You will also have to update the tests to use Test::MockModule to mock read_password in the relevant places.

@cbandy
cbandy commented Aug 27, 2013

iirc, the preferred way to keep passwords private is using a low-privileged file. For PostgreSQL, the PGPASSFILE variable contains a filename. All versions of the MySQL tools can read from a file specified by the --defaults-extra-file argument (n.b. it has to be the first argument) with the following format:

[client]
user = xxx
password = yyy
@theory
Owner
theory commented Aug 27, 2013

Interesting. I don't believe DBD::mysql supports a password file. Such a feature would need to work with DBD::mysql, too.

@theory
Owner
theory commented Sep 4, 2013

Turns out that DBD::mysql does not use ~/.my.cnf. But we might be able to take advantage of MySQL::Config to mimic it.

I am also pondering making use of the MYSQL_PWD environment variable for use with mysql, rather than the --password option. That way it is less likely to appear in the process table. This is just like the current use of the PG_PASSWORD environment variable by the Postgres engine.

@theory theory referenced this issue Oct 8, 2013
@stefansbv stefansbv Fix time zone problem and check againg the first 1/2 of the tutorial
Replace current_timezone with function that add/removes the time zone offset from/to the hour
32ed7ec
@theory
Owner
theory commented Nov 23, 2013

Started on a URI implementation.

@theory
Owner
theory commented Nov 28, 2013

Turns out that the term "target" is already used to name the change one deploys or reverts too. I had forgotten that "designation" was the term I used for where things were deployed to. Not keen on that as a command name, but I think "database" will work quite well, and is, frankly, more obvious. Will update the original description.

@theory
Owner
theory commented Nov 28, 2013

FYI, as part of the URI work I've posted a proposal for a database URI standard.

@theory
Owner
theory commented Nov 29, 2013

Working on this; some notes for myself:

URI precedence rules:

  1. Get URI in command.
  2. If not found, get URI from config file.
  3. If not found, construct from parts in config file. (deprecated)
  4. If command-line-options, override parts in URIs.

Modifications to make:

  • Modify Engine->load to look up based on URI.
  • Add db_uri to Engine with above priorities
  • Remove _engine and engine attributes from Sqitch.pm
  • Add engine() method as sugar for Engine->load. Additional params can be passed (e.g., db_uri)
  • Modify all calls to Sqitch->engine to pass params as appropriate.

Maybe:

  • Move db_client to Engine.
  • Remove all other db_* attributes from Sqitch.pm
  • Add engine pragma to plan file.
@theory
Owner
theory commented Dec 4, 2013

Phase one completed in 7346a30. The engines now all rely on and require a database URI for database connectivity. This actually cuts down on the code a fair bit.

Next up:

  • Add ability to look up URIs by key names in the config.
  • Allow URIs or their key names to be passed to the commands that need them (deploy, revert, rebase, checkout, log, status, and verify).
  • Add database command as sugar for managing the URI key/value pairs.

Need to decide if the database URI configs should be constrained to the current project, or if they can be managed on a user or system basis, as well. Leaning towards allowing it anywhere.

@theory theory closed this Dec 5, 2013
@theory theory reopened this Dec 5, 2013
@theory
Owner
theory commented Dec 8, 2013

Decided to go back to the term target; updated ticket title and description to that effect.

@theory
Owner
theory commented Dec 13, 2013

Merged in the config support, engine integration, and --target option and args in 34c9d03. Now just have to do the target command.

@theory theory added a commit that closed this issue Dec 30, 2013
@theory Merge branch 'target'.
Resolves #100.
fc64af2
@theory theory closed this in fc64af2 Dec 30, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment