Generate Service - RethinkDB Issues

[luke3butler]

I'm not sure if this should be broken into separate issues or not, but they are directly related to RethinkDB.
If these should be separate issues, let me know.

1. When generating an additional RethinkDB service, the existing connection settings will be overwritten with whatever is defined while creating the new service.
2. Service init function does not run when the app is started (tables will not be automatically created).
3. The generator will happily create an invalid database name. Should the generator convert kebab-case to snake_case or CamelCase, or should that be left to the owners responsibility?

For number 2, the solution mentioned in slack would involve modifying the src/index.js file.
The file would go from looking like this:

'use strict';

const logger = require('winston');
const app = require('./app');
const port = app.get('port');
const server = app.listen(port);

process.on('unhandledRejection', (reason, p) =>
  logger.error('Unhandled Rejection at: Promise ', p, reason)
);

server.on('listening', () =>
  logger.info(`Feathers application started on ${app.get('host')}:${port}`)
);

To something like this:

'use strict';

const logger = require('winston');
const app = require('./app');
const port = app.get('port');

process.on('unhandledRejection', (reason, p) =>
  logger.error('Unhandled Rejection at: Promise ', p, reason)
);

let services = Promise.resolve();

Object.keys(app.services).forEach(path => {
  const service = app.service(path);
  if(typeof service.init === 'function') {
    services = services.then(() => service.init());
  }
});

services.then(() => {
  const server = app.listen(port);

  server.on('listening', () =>
    logger.info(`Feathers application started on ${app.get('host')}:${port}`)
  );
});

Of course, there are some other ways this can be handled such as moving the "service initializer" to a separate file and returning the promise. Whatever the solution, I'm not sure if wrapping the app.listen can be avoided.

[daffl]

Closed via #169

[luke3butler]

Hey @daffl #169 does not fix any of these issues... still broken.

[daffl]

Indeed. #172 does though.

[luke3butler]

@daffl I'm guessing that's just the first commit of more to come then, because that alone doesn't fix the issue of creating tables before the server is launched.

It will create the tables and throw an error (because the server launches before the table creation is finished), but then successfully launch on the second attempt (obviously).

error: Unhandled Rejection at: Promise  Promise {
  _bitField: 18087936,
  _fulfillmentHandler0:
   { ReqlRuntimeError
       at Connection._processResponse (/private/var/www/node/feathers-rethink/node_modules/rethinkdbdash/lib/connection.js:395:15)
       at Socket.<anonymous> (/private/var/www/node/feathers-rethink/node_modules/rethinkdbdash/lib/connection.js:201:14)
       at emitOne (events.js:96:13)
       at Socket.emit (events.js:189:7)
       at readableAddChunk (_stream_readable.js:176:18)
       at Socket.Readable.push (_stream_readable.js:134:10)
       at TCP.onread (net.js:551:20)
     msg: 'Table `feathers_rethink.products` does not exist.',
     message: 'Table `feathers_rethink.products` does not exist in:\nr.table("products").changes()\n^^^^^^^^^^^^^^^^^^^          \n',
     frames: [ 0 ],
     name: 'ReqlOpFailedError' },
  _rejectionHandler0: undefined,
  _promise0: undefined,
  _receiver0: undefined } ReqlRuntimeError
    at Connection._processResponse (/private/var/www/node/feathers-rethink/node_modules/rethinkdbdash/lib/connection.js:395:15)
    at Socket.<anonymous> (/private/var/www/node/feathers-rethink/node_modules/rethinkdbdash/lib/connection.js:201:14)
    at emitOne (events.js:96:13)
    at Socket.emit (events.js:189:7)
    at readableAddChunk (_stream_readable.js:176:18)
    at Socket.Readable.push (_stream_readable.js:134:10)
    at TCP.onread (net.js:551:20)

Also, 1. and 3. aren't addressed yet.

[daffl]

So #172 just passed its tests which means it successfully created the connection and service, initialized the database and table and can be accessed via the REST API. Can you create separate issues for anything else that might be outstanding?

As for 3) the default database name should already be kebabcased (https://github.com/feathersjs/generator-feathers/blob/master/generators/connection/index.js#L118), not sure what else should be done for that.

[luke3butler]

The tables are not guaranteed to be created before the server starts listening (I did a bunch of test runs and none of them passed on the first try), so I'll create an issue for that once #172 is merged?

Other issues are broken out.

[daffl]

Does that matter? It would only be an issue at the first server start if something is hitting the API right away in the couple of ms while the server is started and the db and table hasn't yet been created, I'm not sure it's worth worrying about.

[luke3butler]

An ugly error the first time bootstrapping an app, and every time you empty the database, isn't exactly user-friendly. (Edit: Not sure if I was clear that the app crashes... not just an error)

It's your decision, and it's technically functional.

Maybe adding a module.exports.services function (returning the promise) to the rethinkdb.js file would be sufficient so that it can easily be added on by hand.

[daffl]

Ah, I didn't see the error when running the tests. Just fixed it in #172 by not running the parent setup until the init promises are resolved.