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

Cassandra driver #356

Closed
charyorde opened this issue Mar 7, 2016 · 20 comments
Closed

Cassandra driver #356

charyorde opened this issue Mar 7, 2016 · 20 comments
Labels

Comments

@charyorde
Copy link

charyorde commented Mar 7, 2016

^^ https://github.com/charyorde/db-migrate-cassandra

Todos

  • Implement createTable, renameColumn, removeColumn APIs to be fully Cassandra compatible. Need db-migrate-base to take extra options argument.
  • Implement seeders for renameTable. Currently, renaming a table is not supported in Cassandra
  • Add tests
@wzrdtales
Copy link
Member

@charyorde Have already seen it a while ago :)

I also want to make a public directory where community drivers as well as officially supported drivers are listed. I generally want everything more friendly to new users, first steps taken for the documentation (yes I move again, I move away from readthedocs, because of missing search support for mkdocs which is not likely to be fixed anytime soon readthedocs/readthedocs.org#1487).

Keep up the work and let me know if you have some questions, I will be happy to answer.

@charyorde
Copy link
Author

I decided to create a new issue for support purposes.

As stated above, I'll need extra options passed to the createTable, renameColumn, removeColumn base APIs in order to support Cassandra WITH property and AND properties.

For example, ALTER TABLE users WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'min_threshold' : 6 };

Perhaps, this is what the changeColumn API can handle?

@wzrdtales
Copy link
Member

The base api doesn't needs to be adjusted for this, you can always extend and overwrite everything from the base. As soon as you're doing this you will need to document this for your users to make them know of that extended features that are only specific to your driver.

I'm not sure if this is something for changeColumn as you're not really changing a column.

@wzrdtales
Copy link
Member

ALTER TABLE users WITH compaction = { 'class' : 'SizeTieredCompactionStrategy', 'min_threshold' : 6 };

For actions like this, there is very probably a changeTable missing right now. This is something that I will also add for MariaDB and MySQL, to change for example the engine of the table. If you want to add changeTable as an API option, this is also something that will make it into the standard of the API of db-migrate. The definition will be the same as for changeColumn, except for the column param naturally.

@charyorde
Copy link
Author

Noted.

On another note, what triggers the create of migrations table? Is it db-migrate create or db-migrate up

@charyorde
Copy link
Author

This explains it:

function executeUp(internals, config, callback) {

  ...

    migrator.driver.createMigrationsTable(function(err) {
      assert.ifError(err);
      log.verbose('migration table created');

      migrator.up(internals.argv, internals.onComplete.bind(this,
        migrator, callback));
    });
  });
}```

@wzrdtales
Copy link
Member

Y. Just the up, down and some other functions are creating this table, but not create which creates a boilerplate migration.

@charyorde
Copy link
Author

@wzrdtales I can't seem to get the cassandra driver to resolve as expected.

Looking at this line , I was expecting it to lookup the NPM module (db-migrate-cql), but I'm getting the Cannot find module 'db-migrate-cql' error when I run db-migrate up --config database.json -e dev.

The db config file looks like this:

{
  "dev": {
    "keyspace": "db_migrate_test",
    "user": "cassandra",
    "password": "cassandra",
    "driver": {
        "require": "db-migrate-cql"
    },
    "host": "localhost",
    "hosts": ["127.0.0.1"]
  },

  "sql-file": true
}

If I reference the driver locally, it works.

@charyorde
Copy link
Author

Installing globally resolve this: npm i -g db-migrate-cql

@wzrdtales
Copy link
Member

@charyorde This is a configuration schema I am going to deprecate.
https://github.com/db-migrate/node-db-migrate/blob/master/lib/driver/index.js#L45-L46

This circumvents the enforced naming schema. You would configure it like this normally:

{
  "dev": {
    "keyspace": "db_migrate_test",
    "user": "cassandra",
    "password": "cassandra",
    "driver": "cql",
    "host": "localhost",
    "hosts": ["127.0.0.1"]
  },

  "sql-file": true
}

db-migrate will look for the driver in your local project first and after that search globally for it.

@wzrdtales
Copy link
Member

I also just saw that your driver is another cassandra one, thought it would be the one I already known of :)

https://www.npmjs.com/search?q=db-migrate+cassandra

Btw. don't forget to add documentation to your driver, so the users now about methods like createKeyspace are available ;)

@charyorde
Copy link
Author

Cool. "driver": "cql", works as well.

On createKeyspace, does db-migrate support database creation if not exist before start of migration?

@wzrdtales
Copy link
Member

Principially yes, but currently there are known issues that this does not work yet.

@wzrdtales
Copy link
Member

This one here #338

@ramakrishnan
Copy link

HI @wzrdtales,

I did some initial work on getting a Cassandra driver for node-db-migrate. https://github.com/ramakrishnan/db-migrate-cassandra, see if this helps you.

@charyorde
Copy link
Author

@wzrdtales , I need some help with getting out of TTY after the db migrate script is done executing.

Even though I did implemented the close handler as seen at https://github.com/charyorde/db-migrate-cassandra/blob/master/index.js#L297 , but when the db-migrate script finished executing, it gets stuck at

[INFO] Processed migration 20160520102242-add-userstat
INFO] Done

Any ideas on how to get around this please?
Appreciate your help.

@wzrdtales
Copy link
Member

Sure let me quickly setup a docker env for this.

@wzrdtales
Copy link
Member

wzrdtales commented May 23, 2016

@charyorde That error is simple, you're not calling the method to actually close the driver, but you define an event listener. An event that is never going to be called though.

Like this, this can't work though: https://github.com/charyorde/db-migrate-cassandra/blob/master/index.js#L298-L300

Here is a working example of how it needs to be done, to say the cassandra-driver docs are pretty incomplete I needed to look the close function up my self by looking into the source ;):

  close: function(callback) {
    return new Promise( function( resolve, reject ) {

      this.connection.shutdown(function(err) {
        if( err ) {
          return reject( err );
        }
        log.verbose("Cassandra connection closed")
        resolve();
      });
    }.bind(this) )
    .nodeify(callback);
  }

@charyorde
Copy link
Author

Thanks @wzrdtales . Works great. Published a new version with this fix.

@wzrdtales
Copy link
Member

@charyorde Currently cleaning up the issues, I will close here now. Please feel free to hit me up anytime if you've got any questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants