Skip to content

Programmable API createDatabase arguments are overridden #578

Open
@tjokimie

Description

@tjokimie

I'm submitting a...

  • Bug report
    Feature request
    Question

Current behavior

Command line parameters override createDatabase parameters in Programable API.

$ DATABASE_URL=postgres://localhost/postgres node index.js
[INFO] Created database "expected"
$ DATABASE_URL=postgres://localhost/postgres node index.js unexpected
[INFO] Created database "unexpected"

Expected behavior

I would expect parameters provided to createDatabase always override possible command line arguments.

$ DATABASE_URL=postgres://localhost/postgres node index.js
[INFO] Created database "expected"
$ DATABASE_URL=postgres://localhost/postgres node index.js unexpected
[INFO] Created database "expected"

Minimal reproduction of the problem with instructions

$ cat index.js
var dbmigrate = require('db-migrate');
var dbm = dbmigrate.getInstance(true);
dbm.createDatabase('expected')
.then(function() {
  process.exit(0);
});

What is the motivation / use case for changing the behavior?

Using createDatabase through Programable API in an program that has command line args is not possible at the moment.

Environment

db-migrate version: 0.11.1
db-migrate-pg version: 0.4.0

Additional information:
- Node version: v10.6.0
- Platform:  Mac

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

Activity

stale

stale commented on Aug 23, 2018

@stale

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

added and removed on Aug 23, 2018
KingRial

KingRial commented on Sep 17, 2018

@KingRial

I am problably missing something but, shouldn't you just pass the desired configuration ?

This should be the way to handle db-migrate programmatically:

var dbmigrate = require('db-migrate');
var dbm = dbmigrate.getInstance(true, {
  env: 'dev',
  ... all the other options you desire ...
  config: {
   dev: {
     host: 'foo.com',
     user: 'Foo',
     password: 'FooPass',
     database: 'FooMysql',
     driver: 'mysql',
     multipleStatements: true
   }
  }
});
dbm.createDatabase('expected')
.then(function() {
  process.exit(0);
});

Otherwise it correctly looks for configuration used by command line.

tjokimie

tjokimie commented on Sep 17, 2018

@tjokimie
Author

@KingRial Did you try running your example with node index.js and node index.js unexpected? I get the same problem if I pass the configuration directly to getInstance. Command line parameters override parameters provided for createDatabase.

For example with the following script:

var dbmigrate = require('db-migrate');
var config = {
  dev: {
    host: 'localhost',
    user: 'postgres',
    password: 'password',
    database: 'postgres',
    driver: 'pg'
  }
};
var dbm = dbmigrate.getInstance(true, { env: 'dev', config });
dbm.createDatabase('expected')
.then(function() {
  process.exit(0);
});

Running node index.js will create database expected but running node index.js unexpected will create database unexpected.

KingRial

KingRial commented on Sep 17, 2018

@KingRial

Now I understand what happens :) My bad!

I can confirm the problem (it doesn't matter what driver you use; even on mongo and MySQL it occurs)

stale

stale commented on Oct 17, 2018

@stale

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

added and removed on Oct 17, 2018
wzrdtales

wzrdtales commented on Oct 18, 2018

@wzrdtales
Member

I will check this, probably conflicting with the general config hirarchy.

trubachev

trubachev commented on Jul 12, 2019

@trubachev

Faced this bug right now. Found the exact line where it happening

internals.argv.dbname = internals.argv._.shift().toString();

dnjstrom

dnjstrom commented on Jul 12, 2019

@dnjstrom

Quick note: This bug is affecting both createDatabase and dropDatabase since both rely on the executeDB function, as mentioned by @trubachev.

AlexGilleran

AlexGilleran commented on Dec 27, 2019

@AlexGilleran

You can work around it by manually reaching into internals:

dbmigrate.internals.argv._[0] = "test";
await dbmigrate.createDatabase("test");
ajw725

ajw725 commented on Dec 22, 2020

@ajw725

just ran into the same problem. in my case, it came up when trying to run jest tests. i have a package script like this:

{
  "scripts": {
    "test": "NODE_ENV=test jest"
  }
}

and then i have a jest setup script like this:

require('dotenv').config();

const DBMigrate = require('db-migrate');

const dbCreator = DBMigrate.getInstance(true, { env: 'test_no_db' });
const migrator = DBMigrate.getInstance(true, { env: 'test' });

module.exports = async () => {
  console.log('Setting up test database...');
  await dbCreator.createDatabase('my_test_db')
  await migrator.up();
};

it works fine if i just run yarn test, but if i try to run a specific test like yarn test src/some_test.ts, then the script tries to create a database called "src/some_test.ts." i don't think we should assume argv is empty.

ajw725

ajw725 commented on Dec 22, 2020

@ajw725

i don't know if this would be acceptable, but you could do something like:

// api.js
  createDatabase: function (dbname, callback) {
    var executeDB = load('db');
    this.internals.argvOffset = this.internals.argv._.length;
    this.internals.argv._.push(dbname);
    // etc.

and then

// lib/commands/db.js
function executeDB (internals, config, callback) {
  var index = require('../../connect');

  if (internals.argv._.length > 0) {
    var argvOffset = internals.argvOffset || 0;
    for(var i = 0; i < argvOffset; i++) {
      internals.argv._.shift();
    }
    internals.argv.dbname = internals.argv._.shift().toString()
    // etc.

seems to work ok when i monkey-patch it locally that way, but i haven't tested extensively to know if it might somehow break something else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @KingRial@AlexGilleran@dnjstrom@wzrdtales@ajw725

        Issue actions

          Programmable API createDatabase arguments are overridden · Issue #578 · db-migrate/node-db-migrate