-
Notifications
You must be signed in to change notification settings - Fork 11
closes #161 - add Ignite key-value in-memory database #247
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
base: master
Are you sure you want to change the base?
Conversation
src/routes/userRoutes.js
Outdated
Postgres: pgModule.userHasPgAccount, | ||
Elasticsearch: es.createAccount, | ||
Arango: arangoModule.createAccount, | ||
Ignite:igniteModule.createAccount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a missing space between :
and igniteModule.createAccount
src/routes/renderRoutes.js
Outdated
Postgres: process.env.HOST, | ||
Elasticsearch: process.env.ES_HOST, | ||
Arango: process.env.ARANGO_URL, | ||
Ignite:process.env.IGNITE_HOST |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a missing space between :
and process.env.IGNITE_HOST
lib/util.test.js
Outdated
pg.userHasPgAccount = () => true; | ||
es.checkAccount = () => true; | ||
arango.checkIfDatabaseExists = () => true; | ||
ignite.checkAccount = ()=>true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change the format to include spaces?
ignite.checkAccount = () => true
lib/util.js
Outdated
if (arangoDbExists) await arango.deleteAccount(username); | ||
|
||
const igniteDbExists = await ignite.checkAccount(username); | ||
if(igniteDbExists) await ignite.deleteAccount(username); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a space after the if
? For codebase styling consistency. We have a prettierrc.js
file that is supposed to take care of styling. Are you using prettier?
https://github.com/garageScript/databases/blob/master/.prettierrc.js
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use prettier as default vscode formatter usually, so I forgot to check these files before submitting. I run prettier over all database files in new commit and it picked up a few inconsistencies here and there, mostly empty spaces around objects.
Everything should be fixed now.
database/ignite/ignite.js
Outdated
const IgniteClientConfiguration = IgniteClient.IgniteClientConfiguration; | ||
const CacheConfiguration = IgniteClient.CacheConfiguration; | ||
require("dotenv").config(); | ||
let igniteModule = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
igniteModule
should be declared const
instead of let
While Redis supports creating username-password pairs with Access Control List most node.js clients still don't have this feature. Some clients allow you to authorize from node, but creating and deleting users still needs to be done via redis-cli. So Redis is not an option.
The same applies to Memcached and probably most other in-memory databases. Since these databases were designed as caches their security is either non-existent or they support only one global password. Ignite was the only one db with proper authentication out of the box (at least the only one which I could find, there are probably much more). In fact it's more than simple key-value store but in our case it's completely unnecessary.
To safely merge:
IGNITE_HOST
,IGNITE_USER
andIGNITE_PASSWORD
should be added to .env file. Default user-password isignite
, host is127.0.0.1:10800
Start the server with provided configuration
bash ignite.sh path-to-igniteConfig.xml
. It's configured to have persistence (if server runs out of memory heap file on disk will be used, it's also a requirement for authentication for some reason) and to limit in-memory region to 200mb.Tutorial is pretty barebones so far, additions are welcome.