Initialize Database Table #106

kc-dot-io · 2017-05-30T18:59:36Z

overview

This PR adds service({}).init() which returns a promise and gracefully degrades if database is already initialized.

tasks

kc-dot-io · 2017-05-30T19:38:23Z

@daffl @marshallswain @ekryski @eddyystop - This is ready if anyone wants to give it a look over. Same idea as this one on feathers-rethinkdb - makes it much easier to abstract the database models in the generator if we use this method so people can define their own schemas.

update readme readme spacing Readme comments

kc-dot-io · 2017-05-30T20:20:44Z

Seems like an issue upstream from us with this part of the pre-gyp / sqlite3 build on node 8:

node-pre-gyp http GET https://fsevents-binaries.s3-us-west-2.amazonaws.com/v1.1.1/fse-v1.1.1-node-v57-darwin-x64.tar.gz
node-pre-gyp http 404 https://fsevents-binaries.s3-us-west-2.amazonaws.com/v1.1.1/fse-v1.1.1-node-v57-darwin-x64.tar.gz
node-pre-gyp ERR! Tried to download(404): https://fsevents-binaries.s3-us-west-2.amazonaws.com/v1.1.1/fse-v1.1.1-node-v57-darwin-x64.tar.gz
node-pre-gyp ERR! Pre-built binaries not found for [email protected] and [email protected] (node-v57 ABI) (falling back to source compile with node-gyp)
node-pre-gyp http 404 status code downloading tarball https://fsevents-binaries.s3-us-west-2.amazonaws.com/v1.1.1/fse-v1.1.1-node-v57-darwin-x64.tar.gz
node-pre-gyp verb command build [ 'rebuild' ]
gyp WARN download NVM_NODEJS_ORG_MIRROR is deprecated and will be removed in node-gyp v4, please use NODEJS_ORG_MIRROR
gyp WARN download NVM_NODEJS_ORG_MIRROR is deprecated and will be removed in node-gyp v4, please use NODEJS_ORG_MIRROR
gyp WARN download NVM_NODEJS_ORG_MIRROR is deprecated and will be removed in node-gyp v4, please use NODEJS_ORG_MIRROR

kc-dot-io · 2017-05-30T20:28:22Z

I am not sure this is something we can solve until upstream compiles a node 8 compatible version of the fsevents pre-built binaries or fixes the node-gyp build.

kc-dot-io · 2017-05-30T21:12:16Z

Posted in node-sqlite3 to see anyone can point me in the right direction.

kc-dot-io · 2017-05-30T23:17:16Z

Yay, build fixed in TryGhost/node-sqlite3#825!

daffl · 2017-06-21T03:57:01Z

Sorry, totally missed this one somehow. Is this because of the same issue as feathersjs/feathers#581 (and the generator fix in feathersjs-ecosystem/generator-feathers#231)?

kc-dot-io · 2017-06-21T04:04:11Z

@daffl it refactors that logic into init() just like we did in feathers-rethinkdb and I intend to do in every adapter for v3 generator. I wasn't aware of either of those issues but they seem related, if not overlapping. This addresses th by moving the logic to init, which can be conditional and I think is an ideal approach so we keep the adapter interfaces consistent. More in person on Thursday?

kc-dot-io · 2017-06-21T23:06:21Z

@daffl are you cool with me merging and publishing this as 2.7.0?

daffl · 2017-06-21T23:12:12Z

Two things:

Just like the feathers-rethinkdb init it should probably not blow away an existing database, just create it if it doesn't exist
Everything should return Promises, not use callbacks anymore

kc-dot-io · 2017-06-21T23:31:40Z

@daffl it doesn't look like this is supported by knex:

Unhandled rejection TypeError: A callback function must be supplied to calls against `.createTable` and `.table`
    at new TableBuilder (/Users/slajax/repos/feathers/feathers-knex/node_modules/knex/lib/schema/tablebuilder.js:47:11)
    at Client_SQLite3.tableBuilder (/Users/slajax/repos/feathers/feathers-knex/node_modules/knex/lib/client.js:143:12)
    at SchemaCompiler_SQLite3.createTableIfNotExists (/Users/slajax/repos/feathers/feathers-knex/node_modules/knex/lib/schema/compiler.js:59:31)
    at SchemaCompiler_SQLite3.toSQL (/Users/slajax/repos/feathers/feathers-knex/node_modules/knex/lib/schema/compiler.js:51:26)
    at SchemaBuilder.toSQL (/Users/slajax/repos/feathers/feathers-knex/node_modules/knex/lib/schema/builder.js:57:43)
    at /Users/slajax/repos/feathers/feathers-knex/node_modules/knex/lib/runner.js:56:32
From previous event:
    at Runner.run (/Users/slajax/repos/feathers/feathers-knex/node_modules/knex/lib/runner.js:51:31)
    at SchemaBuilder.Target.then (/Users/slajax/repos/feathers/feathers-knex/node_modules/knex/lib/interface.js:35:43)
    at Service.init (/Users/slajax/repos/feathers/feathers-knex/lib/index.js:103:53)
    at Object.<anonymous> (/Users/slajax/repos/feathers/feathers-knex/example/app.js:37:4)
    at Module._compile (module.js:570:32)
    at loader (/Users/slajax/repos/feathers/feathers-knex/node_modules/babel-register/lib/node.js:144:5)
    at Object.require.extensions.(anonymous function) [as .js] (/Users/slajax/repos/feathers/feathers-knex/node_modules/babel-register/lib/node.js:154:7)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Function.Module.runMain (module.js:604:10)
    at Object.<anonymous> (/Users/slajax/repos/feathers/feathers-knex/node_modules/babel-cli/lib/_babel-node.js:154:22)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Module.runMain (module.js:604:10)

daffl · 2017-06-21T23:56:07Z

It should work as long as you call .then (something like createTable().then(res => res)). Not sure why their promises are lazy but that should do it. Also, I think createTableIfNotExists has a bug (see feathersjs/feathers#581) so we'll probably need the same workaround as in feathersjs-ecosystem/generator-feathers#231 using .hasTable first.

kc-dot-io · 2017-06-22T01:27:01Z

This has been updated so that it returns a promise and doesn't drop the table if the table already exists. It still however requires us to pass a callback into init({}, cb) because knex needs to be able to define the schema when it creates the table. The cb is required as per the error in the convo above.

Fundamentally this is the same thing we're doing in v2, but more user friendly because we moved the table management decision into the init method so user land doesn't have to fumble around with the best way to manage the table. This is particularly important within the generator. We should adopt this in v2 generator templates too.

kc-dot-io · 2017-06-22T01:27:28Z

I think this is ready to be released now.

kc-dot-io force-pushed the slajax/init branch from 0816b43 to d5a8b81 Compare May 30, 2017 19:15

kc-dot-io changed the title ~~Initialize Databaes~~ Initialize Database Table May 30, 2017

kc-dot-io added 3 commits May 30, 2017 12:46

.init({}, fn) for initializing db schema

f6ddb6d

update readme readme spacing Readme comments

update example

f2ad38c

fix tests

ee0f2a7

kc-dot-io force-pushed the slajax/init branch from f8091ee to ee0f2a7 Compare May 30, 2017 19:46

kc-dot-io added 4 commits May 30, 2017 13:34

try to fix build

060218f

add node 7, separate from node 8

4b85c8a

try build from source && update sqlite3

da46d93

remove troubleshooting

5ef7704

This was referenced May 30, 2017

Postgres Support feathersjs-ecosystem/feathers-generator#58

Merged

SQLite Support feathersjs-ecosystem/feathers-generator#60

Merged

Mysql Support feathersjs-ecosystem/feathers-generator#61

Merged

kc-dot-io added 2 commits June 21, 2017 16:44

only create table if it does not exist

d4514a7

refactor callback

0db3111

kc-dot-io added 2 commits June 21, 2017 17:47

fix issue with dropTableIfExists and return promise

d42ff78

fix tests

edeecb4

daffl merged commit 1946917 into master Jun 22, 2017

daffl deleted the slajax/init branch June 22, 2017 02:22

daffl mentioned this pull request Jun 22, 2017

Minor init method refactoring #107

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initialize Database Table #106

Initialize Database Table #106

kc-dot-io commented May 30, 2017 •

edited

Loading

kc-dot-io commented May 30, 2017

kc-dot-io commented May 30, 2017 •

edited

Loading

kc-dot-io commented May 30, 2017 •

edited

Loading

kc-dot-io commented May 30, 2017

kc-dot-io commented May 30, 2017

daffl commented Jun 21, 2017 •

edited

Loading

kc-dot-io commented Jun 21, 2017

kc-dot-io commented Jun 21, 2017

daffl commented Jun 21, 2017 •

edited

Loading

kc-dot-io commented Jun 21, 2017 •

edited

Loading

daffl commented Jun 21, 2017

kc-dot-io commented Jun 22, 2017

kc-dot-io commented Jun 22, 2017

Initialize Database Table #106

Initialize Database Table #106

Conversation

kc-dot-io commented May 30, 2017 • edited Loading

overview

tasks

kc-dot-io commented May 30, 2017

kc-dot-io commented May 30, 2017 • edited Loading

kc-dot-io commented May 30, 2017 • edited Loading

kc-dot-io commented May 30, 2017

kc-dot-io commented May 30, 2017

daffl commented Jun 21, 2017 • edited Loading

kc-dot-io commented Jun 21, 2017

kc-dot-io commented Jun 21, 2017

daffl commented Jun 21, 2017 • edited Loading

kc-dot-io commented Jun 21, 2017 • edited Loading

daffl commented Jun 21, 2017

kc-dot-io commented Jun 22, 2017

kc-dot-io commented Jun 22, 2017

kc-dot-io commented May 30, 2017 •

edited

Loading

kc-dot-io commented May 30, 2017 •

edited

Loading

kc-dot-io commented May 30, 2017 •

edited

Loading

daffl commented Jun 21, 2017 •

edited

Loading

daffl commented Jun 21, 2017 •

edited

Loading

kc-dot-io commented Jun 21, 2017 •

edited

Loading