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

Couchbase Driver - Need some questions answered #380

Closed
moxious opened this issue Jun 20, 2016 · 9 comments
Closed

Couchbase Driver - Need some questions answered #380

moxious opened this issue Jun 20, 2016 · 9 comments
Assignees
Labels

Comments

@moxious
Copy link

moxious commented Jun 20, 2016

I'm mid-way through writing the couchbase driver. I've got working code in a local test environment, I'm working on building a better test suite runnable through CircleCI to ensure the safety of it.

https://github.com/etops/db-migrate-couchbase

I could use some help though in the form of some questions which are causing troubles getting this ready:

  • Through the use of Base.extend, it seems the db-migrate API has some strange rules on what it exposes. I write some couchbase-specific functions in my driver, for example runN1ql as the query language that DB uses. Now, I can override runSql and then in migrations that function is available, no problem; but if I were to attempt to run the runN1ql function on my driver, it fails saying that isn't a function. Is there some special logic being applied about which functions the driver exports and makes publicly callable? Non-relational databases may require a different API per-database.
  • What is the recommended method of testing a driver? Right now, I'm installing the driver into its own node_modules folder and I intend to run sample migrations on a live DB as part of the testing process. I haven't yet succeeded with a build but the path to doing this seems pretty clear.
@wzrdtales
Copy link
Member

wzrdtales commented Jun 20, 2016

@moxious Great to finally also see a couchbase driver :)

Ok, sry this is going to get a bit longer now.

Interfaces

To answer your first question, yes there is some strange magic applied here, actually quite some, there are also things like shadow drivers intended for self learning capabilities. This is due to some upcoming concepts enforcing certain rules about what a driver actually exposes to a seeder and what it does expose to a migration.

There are some simple rules:

  • Everything considered private, which in node always starts with an underscore is not affected of this and is left as it is
  • A driver might only expose what it is intended to actually expose, but also can define and or override this

Extending Interfaces

To give you one example of how a driver can define something that should be exposed aside from what is already defined by the current interfaces:

https://github.com/db-migrate/mysql/blob/9cac0862d1a6333f9d1d22098a08705e84a507f5/index.js#L409

At this point the mysql driver adds a function to the SeederInterface. This is actually from a time before private variables and functions have been ignored.

You can do the same thing for the MigratorInterface.

Actually Interfaces do need some love though, I think it is more a thing of the driver to define the two interfaces and not of db-migrate itself. Basically the goal was to only provide the toolset to a migration and seeder that is actually intended for them. It would be kind of strange to have data definition tools at a hand within a seeder.

You can find these interfaces over here:
https://github.com/db-migrate/node-db-migrate/blob/master/lib/interface/migratorInterface.js

What you get passed in your driver setup function are actually the extension definitions:
https://github.com/db-migrate/node-db-migrate/blob/master/lib/driver/index.js#L29-L33

Testing

The recommended testing is to actually test every possible API input with its expected result. I do suggest on this layer to do integration tests and some kind of unit tests.
Integration tests are interesting for drivers as those also prove that they work when there are new releases of the database which might have changed some important detail, although that this is very uncommon.

On the other side you can directly test the query generation, if a specified input ends up in the expected query.

@wzrdtales wzrdtales self-assigned this Jun 20, 2016
@moxious
Copy link
Author

moxious commented Jun 21, 2016

Thanks for this; I'll be hacking on this driver tonight and tomorrow, and should have an update soon. I'll play with the interfaces first, then work on some testing. I think the driver is not actually that far away from ready; I am actually finding setting up representative test cases, integration tests and so on to be more challenging than writing the driver itself. I plan to use this on a production system though, so I need the tests to be good and representative as the consequences for messing up a migration are fairly substantial.

What's the best way to communicate status on this? I posted an issue here because I saw others do this; do you prefer to keep this issue open even though it's not an actual bug report about db-migrate per se or is there some other forum/list that's more appropriate for talking details?

@wzrdtales
Copy link
Member

I don't mind open issues, they are a good way to keep things transparent, leave this one open and continue communicating here.

@moxious
Copy link
Author

moxious commented Jun 21, 2016

Putting new methods on the interface seems not to be effective, I'm wondering if there's something I'm missing in what you wrote above? I have this code, it seems to result in my methods being defined but returning the default method.

The way I interpreted what you were saying, in the mysql example you set the function to dummy because it would then be part of the interface, then overridden by the driver spec. Is that right?

  /* Export functions to the interface that are not normally part
   * of the migration interface.
   */
  const exportable = [
    'runN1ql', 'registerModel', 'getModel', 'createOttomanModel',
    'renameOttomanModel', 'addOttomanPath', 'removeOttomanPath',
    'renameOttomanPath', 'createBucket', 'dropBucket',
    'withBucket', 'getBucketNames', 'getIndexes',
  ];

  exportable.forEach(f => {
    /* eslint no-param-reassign: "off" */
    intern.interfaces.MigratorInterface[f] = function () {
      return new Promise((resolve, reject) => {
        return reject('Not implemented');
      }).nodeify(arguments[arguments.length -1]);
    };
    return f;
  });

If I do the above, and then call some of those exportable functions in a migration, I get this "Not implemented" implementation, not the overridden one from the driver I'd expect. Any thoughts?

@moxious
Copy link
Author

moxious commented Jun 21, 2016

Another issue I'm running into is for mocha testing, how to get an instance of my own driver for testing. I'm using the docs from the Programmable API page but they appear incomplete; if I call DBMigrate.getInstance with a callback, (to get my migrator) the callback never fires. On the other hand if I use the return value from getInstance (rather than what comes by the callback) then it doesn't have my methods defined on it.

So how do you get an instance of your own driver programmatically for mocha testing?

@wzrdtales
Copy link
Member

Let me get into this, will come back to you shortly.

@wzrdtales
Copy link
Member

First of all return Promise.reject() is probably a more elegant way of what you shown above :)

I will get onto your question about the testing first.

@wzrdtales
Copy link
Member

wzrdtales commented Jun 21, 2016

So about the programable API, yes the docs are not final yet about this.

https://github.com/db-migrate/api-examples/blob/master/index.js

That would be an example of a proper usage of the programable API. But you should not necessarily need to access that for basic testing.

Let's take this driver as example:

https://github.com/db-migrate/node-db-migrate/blob/master/test/driver/mysql_test.js

Basically the only two components you really need from db-migrate are actually log and driver, but also the latter one not necessarily.

Actually the driver.connect does not do that much, it does require the specified driver and calls it setup functions passing along some db-migrate specific things.

So what do I think of this right now?
I see the problems you have I would go a step ahead and say lets change some things of how and what db-migrate offers to actually easen the process of testing. I wanted to move all the tests currently still in the main project to their drivers anyway, so it is a good thing to have that part done.

I will check out your driver and give you a hand on doing this things and improve some details about the current behavior of db-migrate itself. So I'm getting settled for this this evening and let you know some status updates as soon as I have some for you, I will first grab something to eat now.

@wzrdtales
Copy link
Member

@moxious ment to close this one instead :)

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

2 participants