Remove leveldown dependency about nano-sql HOT 19 CLOSED

Hello Heri,
I updated the api again for custom leveldown adapters, the example below should work with 1.6.7

const { nSQL } = require("nano-sql");
const { _LevelStore } = require("nano-sql/lib/database/adapter-levelDB");
const rocksdb = require("rocksdb");
const fs = require("fs");
const path = require("path");

// Declare Table
nSQL('posts').model([
    { key: 'id', type: 'int', props: ['pk', 'ai'] },
    { key: 'title', type: 'string' },
    { key: 'content', type: 'string' },
    { key: 'date', type: 'int' },
]).config({
    id: 'eqh', // namespace of the underlying table
    mode: new _LevelStore((databaseID, tableName) => {
        const basePath = path.join(__dirname, "db_" + databaseID);
        if (!fs.existsSync(basePath)) {
            fs.mkdirSync(basePath);
        }
        return {
            lvld: rocksdb(path.join(basePath, tableName)),
            args: {} // arguments object to pass into LevelUp function as second argument.
        };
    }),
    history: 'row', // store each row's changes as a revision history
    // mode: 'PERM', // store changes permenantly
    // cache: false, // dont use javascript object cache
}).connect().then(() => {
    return nSQL().query("upsert", {title: "hello"}).exec();
}).then(() => {
    return nSQL().query("select").exec();
}).then((rows) => {
    console.log(rows);
})

I totally get not wanting to deal with leveldown in the build process if you're not using it, the alternatives provided would inconvenience current users of the library in ways I'd like to avoid. For example, peerDependencies don't get installed by default as you suggested meaning someone would have to run npm i levelup before using the LevelDB features. If we're going to do that, might as well split it out into it's own adapter library so the behavior is more verbose and consistent with the rest of the library.

Still, I'd like to keep around mode: "PERM" working in NodeJS without any other installs needed so dropping LevelDB from the package we'd need to add an alternative of some kind back in.

How often do you call npm rebuild? My understanding is this process is needed infrequently. It might be more reasonable to put together a small script that prunes the leveldown dependency from nanoSQL in your node_modules folder after install, something like this:

const fs = require("fs");
const path = require("path");

const packageJSON = path.join(__dirname, "node_modules", "nano-sql", "package.json");
fs.readFile(packageJSON, (err, file) => {
    if (err) throw err;
    const package = JSON.parse(file.toString());
    delete package.dependencies.leveldown;
    fs.writeFile(packageJSON, JSON.stringify(package, null, 4), (err) => {
        if (err) throw err;
    })
});

Still, I wouldn't mind leaving this issue open for a while and seeing if other folks have feedback. If you're experience is a common one, that is people would prefer to do the extra step to get LevelDB rather than have it bundled I'm certainly open to it.

from nano-sql.

Interesting, I didn't know you could use optional dependencies like that.

I think that makes everyone happy then, 1.6.8 was just pushed to NPM and includes all the NodeJS dependencies moved to the optionalDependencies property of package.json. 👍

from nano-sql.

Alright, i was wrong on this, nodejs module resolution seems alright.

No LevelStore

npm install --no-optional nano-sql

RocksDB

npm install int64-buffer levelup@2
npm install --no-optional nano-sql
npm install rocksdb

from nano-sql.

LevelDown doesn't affect the size of webpack/browser bundles and the like, node/webpack bundle a different index file for the server and browser, the browser version doesn't include the LevelDown dependencies.

As for removing it from the package.json as a dependency to reduce the download size for installs, I think the downsides to doing that far outweigh any positives. You wouldn't be able to just "plug and play" nanoSQL with NodeJS and get a persistent database anymore. This would also break anyone installing the update from a fresh install since LevelDown would no longer be downloaded like it was before (which is now expected behavior).

All that being said I just pushed 1.6.4, you can use RocksDB or any other LevelDown compatible adapter with the below method:

import { _LevelStore } from "nano-sql/lib/database/adapter-levelDB";
import * as LevelDown from "leveldown";

nSQL()
... data models and such
.config({
    mode: new _LevelStore("", 0, 0, LevelDown())
})
.connect()..

Let me know if you run into any further issues!

from nano-sql.

Since there's been no response to this issue for a few days I'm going to close it. Feel free to open it back up if you have further questions.

from nano-sql.

npm rebuild on my project takes a long time due to leveldown, even though I don't intend to use leveldown.

Could we at least move it into peer Dependencies which seem to be an idiomatic way in npm@6 land (https://docs.npmjs.com/files/package.json#peerdependencies).

• People who would like to use the defaults will automatically see a warning by npm to install leveldown.
• People who would not like to use the default (e.g. use Google Datastore) will just create a stub package on github that is named leveldown but contains nothing. if leveldown is also added into optionalDependencies, this step might not even be required.

We could also add npm install express nano-sql leveldown to the example documentation.

from nano-sql.

I don't think OP have been given the rights to reopen issues.

from nano-sql.

@ClickSimply the suggested code did not work for me.

Sample code:

const { nSQL } = require('nano-sql')
const { _LevelStore } = require('nano-sql/lib/database/adapter-levelDB')
const rocksdb = require('rocksdb')
const leveldown = require('leveldown')

const fs = require('fs')

if (!fs.existsSync('/tmp/db_eqh')) {
  fs.mkdirSync('/tmp/db_eqh')
}

// Declare Table
nSQL('posts').model([
  { key: 'id', type: 'int', props: ['pk', 'ai'] },
  { key: 'title', type: 'string' },
  { key: 'content', type: 'string' },
  { key: 'date', type: 'int' },
]).config({
  id: 'eqh', // namespace of the underlying table
  mode: new _LevelStore('/tmp', 32, 32, rocksdb('/tmp/db_eqh/posts')), // AWS Lambda only has one writable directory
  history: 'row', // store each row's changes as a revision history
  // mode: 'PERM', // store changes permenantly
  // cache: false, // dont use javascript object cache
})

Error:

OpenError: IO error: lock /tmp/db_eqh/posts/LOCK: No locks available
OpenError: IO error: lock /tmp/db_eqh/posts/LOCK: already held by process

from nano-sql.

The prune script sounds like a great idea.

Another nice way is to move leveldown into optionalDependencies and let users do npm install —no-optional .
See: https://docs.npmjs.com/cli/install

from nano-sql.

Hi @ClickSimply, I have a feeling that by including levelup into the optionalDependencies, you would also need to add it into peerDependencies too. If not how would nodejs resolve the path to levelup and int64-buffer when npm install --no-optional is used?

peerDependencies: {
  "levelup": "^2.0.2",
  "int64-buffer": "^0.1.10"
}

├─┬ [email protected]
│ ├── [email protected]
│ ├── UNMET OPTIONAL DEPENDENCY [email protected]
│ ├─┬ UNMET OPTIONAL DEPENDENCY [email protected]
│ │ ├── [email protected] deduped
│ │ ├── [email protected] deduped
│ │ ├── [email protected] deduped
│ │ ├── [email protected] deduped
│ │ └── [email protected] deduped
│ ├─┬ UNMET OPTIONAL DEPENDENCY [email protected]
│ │ ├─┬ UNMET OPTIONAL DEPENDENCY [email protected]
│ │ │ └─┬ UNMET OPTIONAL DEPENDENCY [email protected]
│ │ │   └── [email protected] deduped
│ │ ├─┬ UNMET OPTIONAL DEPENDENCY [email protected]
│ │ │ └─┬ UNMET OPTIONAL DEPENDENCY [email protected]
│ │ │   └── UNMET OPTIONAL DEPENDENCY [email protected]
│ │ ├─┬ UNMET OPTIONAL DEPENDENCY [email protected]
│ │ │ ├── [email protected] deduped
│ │ │ ├── [email protected] deduped
│ │ │ └── [email protected] deduped
│ │ └── [email protected] deduped
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├── [email protected]
│ ├─┬ [email protected]
│ │ └── [email protected]
│ ├── [email protected]
│ └── [email protected]

Let me just check on this and get back to you.

from nano-sql.

When leveldown not installed, crashed due to https://github.com/ClickSimply/Nano-SQL/blob/094cab8484bfe98947f7018cdbf8eaf5b3d9d3be/lib/index-server.js#L5

try {
  global._levelup = require("levelup");
} catch (err) {}
try {
  global._leveldown = require("leveldown");
} catch (err) {}
try {
  global._Int64BE = require("int64-buffer").Uint64BE;
} catch (err) {}

Would you like me to send a PR instead?

from nano-sql.

Ah forgot about that. Sure, send it through and we'll hopefully get to close this issue for good soon! 😆

from nano-sql.

Do we need a documentation update on the README?

const fs = require('fs')
const path = require('path')

const { nSQL } = require('nano-sql')
const { _LevelStore: LevelStore } = require('nano-sql/lib/database/adapter-levelDB')
const rocksdb = require('rocksdb')

nSQL().config({
  id: 'eqh', // namespace of the underlying table
  history: 'row', // store each row's changes as a revision history
  // dbPath: '/tmp', // Not required
  mode: new LevelStore((databaseID, tableName) => {
    const basePath = path.join('/tmp', `db_${databaseID}`)
    if (!fs.existsSync(basePath)) {
      fs.mkdirSync(basePath)
    }
    return {
      lvld: rocksdb(path.join(basePath, tableName)),
      // arguments to pass into LevelUp function as second argument.
      args: {
        cacheSize: 32 * 1024 * 1024,
        writeBufferSize: 32 * 1024 * 1024,
      },
    }
  })
})

from nano-sql.

@ClickSimply Would you consider using level-packager instead to avoid the excessive custom configuration?

Exports a single function which takes a single argument, an abstract-leveldown compatible storage back-end for levelup. The function returns a constructor function that will bundle levelup with the given abstract-leveldown replacement.

Example code here: https://github.com/Level/level-rocksdb

const mode = require('rocksdb')
const levelFunc = require('level-packager')(mode)

from nano-sql.

1.6.9 is live on NPM with the PR. 👍

from nano-sql.

Closing this issue again, let me know if we still need to adjust things.

from nano-sql.

Heya, I know Rocksdb is great performance wise, but I think having a build-in adapter that has native code shouldn't be part of the core. I, for example, have been bitten by

dyld: lazy symbol binding failed: Symbol not found: __ZN2v816FunctionTemplate3NewEPNS_7IsolateEPFvRKNS_20FunctionCallbackInfoINS_5ValueEEEENS_5LocalIS4_EENSA_INS_9SignatureEEEiNS_19ConstructorBehaviorE
  Referenced from: /Users/bkniffler/Git/diego/node_modules/rocksdb/build/Release/leveldown.node
  Expected in: flat namespace

dyld: Symbol not found: __ZN2v816FunctionTemplate3NewEPNS_7IsolateEPFvRKNS_20FunctionCallbackInfoINS_5ValueEEEENS_5LocalIS4_EENSA_INS_9SignatureEEEiNS_19ConstructorBehaviorE
  Referenced from: /Users/bkniffler/Git/diego/node_modules/rocksdb/build/Release/leveldown.node
  Expected in: flat namespace

And when switching to SQLite, my electronjs will always still build the Rocksdb native dependency.

So wouldn't it maybe make sense to have only memory as a default adapter?

from nano-sql.

Oh, I could only bypass the error by referencing '@nano-sql/core/lib/index'

import { SQLite } from '@nano-sql/adapter-sqlite';
import { nSQL } from '@nano-sql/core/lib/index';

from nano-sql.

The solution mentioned in this thread should also work for v2:

npm install --no-optional @nano-sql/core

All of the NodeJS specific dependencies are set as optional. Also pulling the index file used for browser builds as you suggested is another good solution.

from nano-sql.