4 ms·

If you are going to invoke the sacred name of 'sqlite', then I expect to see rock-solid code and quality testing. Instead, there are two tiny tests, and I foun

If you are going to invoke the sacred name of 'sqlite', then I expect to see rock-solid code and quality testing. Instead, there are two tiny tests, and I found a couple of issues in 30 seconds of looking at the code. EDIT: I should prove my point. 1) Race condition on calculating uuid (obviously won't be too serious for a good uuid implementation) 2) No check that file is written successfully. 3) No check when json is invalid, just silently swallowed, or filesystem full. 4) Makes one file a json file, will scale terribly past a few thousand json files.

nodesocket 10y ago
Create some issues and pull requests and help out. :-)
- rad_gruchalski 10y ago
  How about some due diligence first? I know, it's so easy to say "just create a pull request". To which the answer is: "I'm not going to use your software if you can't make effort to test it and document properly".
- throwanem 10y ago
  Show me why your project is worth some of my finite time and effort, and I might. This looks like a toy project to me, and while there's nothing wrong with a toy project, I'm not seeing why it's more fun for me to play with this toy than some other of the myriad from which I have to choose.
nodesocket 10y ago
Good point on 2 and 3. Should just involve checking the previous commands return code before echoing the uuid. I'll get that patched up and ship a new version in a bit. Not sure what you mean exactly by 1 and 4 though.
- jakewins 10y ago
  Many operating systems do not handle tens of thousands of file handles open simultaneously very well. For instance, if you run this database on Linux, the RAM overhead for each document is 1K just for the file handle, see the comment at the bottom here http://lxr.free-electrons.com/source/fs/file_table.c http://lxr.free-electrons.com/source/fs/file_table.c
- CJefferson 10y ago
  With 1, you are trusting the directory returned by uuid will be unique --it should be, but it might not be, in particular if it is based on time and I run in two threads. With 4, I find once a directory has about 100,000 files in it, things get bad, from the simple (ls * won't work) to the nastier (git starts using huge amounts of space, you'll hit github's size limit even though all your files are quite small).
  - avip 10y ago
    In all fairness, I find this (and only this) criticism unsubstantiated/nitpicky. Many systems are relying on the statistical improbability of 32-hexdigits uuid collision (name-dropping one - Disque).
  - sz4kerto 10y ago
    I would not let anyone close to the code I'm working on if he wanted to verify whether a properly generated UUID clashes with some previously generated UUID. You could just as do stuff like "int x=1; assert(x == 1);" and so on. And it would be just as futile.
    - CJefferson 10y ago
      In this case, you are relying on what installed program or function is called when you invoke uuid in the shell. It seems reasonable to me to check if it is having sensibly, if the reliability of your program counts on it. I have no idea what dodgy uuid programs you might find installed.
- tyingq 10y ago
  On 4, the point might be that thousands of files in a single directory can create performance issues with readdir(). The usual workaround to that is to create subdirectories based on the first few characters of the filename. Since you're using bash, you could do something like: $ echo 1df8be33-4392-471a-99af-3df967b87cb6 |sed -e 's/^$.$$.$/\1\/\2\/\1\2/' 1/d/1df8be33-4392-471a-99af-3df967b87cb6 Edit: You may want more than 2 levels of directories, or directories with 2 character names instead of 1, etc. Apache's mod_cache defaults to 2 levels of 2 character names, but the filenames are base 64, so more possibilities than hex.
nodesocket 10y ago
Heads up, just fixed the bug where providing invalid json to set would write an empty file with a uuid filename to the data directory. Now providing invalid json to set errors with no file written to the data directory. New version is 1.1.0 (https://github.com/nodesocket/jsonlite/releases/tag/1.1.0 https://github.com/nodesocket/jsonlite/releases/tag/1.1.0) if you care.
- CJefferson 10y ago
  Just wanted to say, glad to see these are being fixed. My initial message was a little harsh -- although github makes it hard to tell if you are making a new library for fun, or something you really want people using for important customer data!

nodesocket 10y ago

Create some issues and pull requests and help out. :-)

rad_gruchalski 10y ago
How about some due diligence first? I know, it's so easy to say "just create a pull request". To which the answer is: "I'm not going to use your software if you can't make effort to test it and document properly".
throwanem 10y ago
Show me why your project is worth some of my finite time and effort, and I might. This looks like a toy project to me, and while there's nothing wrong with a toy project, I'm not seeing why it's more fun for me to play with this toy than some other of the myriad from which I have to choose.

nodesocket 10y ago

Good point on 2 and 3. Should just involve checking the previous commands return code before echoing the uuid. I'll get that patched up and ship a new version in a bit. Not sure what you mean exactly by 1 and 4 though.

jakewins 10y ago
Many operating systems do not handle tens of thousands of file handles open simultaneously very well. For instance, if you run this database on Linux, the RAM overhead for each document is 1K just for the file handle, see the comment at the bottom here http://lxr.free-electrons.com/source/fs/file_table.c http://lxr.free-electrons.com/source/fs/file_table.c
CJefferson 10y ago
With 1, you are trusting the directory returned by uuid will be unique --it should be, but it might not be, in particular if it is based on time and I run in two threads. With 4, I find once a directory has about 100,000 files in it, things get bad, from the simple (ls * won't work) to the nastier (git starts using huge amounts of space, you'll hit github's size limit even though all your files are quite small).
- avip 10y ago
  In all fairness, I find this (and only this) criticism unsubstantiated/nitpicky. Many systems are relying on the statistical improbability of 32-hexdigits uuid collision (name-dropping one - Disque).
- sz4kerto 10y ago
  I would not let anyone close to the code I'm working on if he wanted to verify whether a properly generated UUID clashes with some previously generated UUID. You could just as do stuff like "int x=1; assert(x == 1);" and so on. And it would be just as futile.
  - CJefferson 10y ago
    In this case, you are relying on what installed program or function is called when you invoke uuid in the shell. It seems reasonable to me to check if it is having sensibly, if the reliability of your program counts on it. I have no idea what dodgy uuid programs you might find installed.
tyingq 10y ago
On 4, the point might be that thousands of files in a single directory can create performance issues with readdir(). The usual workaround to that is to create subdirectories based on the first few characters of the filename. Since you're using bash, you could do something like: $ echo 1df8be33-4392-471a-99af-3df967b87cb6 |sed -e 's/^$.$$.$/\1\/\2\/\1\2/' 1/d/1df8be33-4392-471a-99af-3df967b87cb6 Edit: You may want more than 2 levels of directories, or directories with 2 character names instead of 1, etc. Apache's mod_cache defaults to 2 levels of 2 character names, but the filenames are base 64, so more possibilities than hex.

nodesocket 10y ago

Heads up, just fixed the bug where providing invalid json to set would write an empty file with a uuid filename to the data directory. Now providing invalid json to set errors with no file written to the data directory. New version is 1.1.0 (https://github.com/nodesocket/jsonlite/releases/tag/1.1.0 https://github.com/nodesocket/jsonlite/releases/tag/1.1.0) if you care.

CJefferson 10y ago
Just wanted to say, glad to see these are being fixed. My initial message was a little harsh -- although github makes it hard to tell if you are making a new library for fun, or something you really want people using for important customer data!