Allow for specification of dat secret storage directory. #193
Conversation
|
Thanks! I moved the secret keys to an option in the second argument. Can you update this PR to reflect that? Should make it a bit simpler =). It'd be great to have a test for this as well! Please also make sure you run |
brandonedens
force-pushed
the
specify-secret-storage-dir
branch
from
95394d7
to
5db3f06
Compare
Codecov Report
@@ Coverage Diff @@
## master #193 +/- ##
==========================================
- Coverage 84.86% 84.41% -0.46%
==========================================
Files 7 7
Lines 304 308 +4
==========================================
+ Hits 258 260 +2
- Misses 46 48 +2
Continue to review full report at Codecov.
|
brandonedens
force-pushed
the
specify-secret-storage-dir
branch
2 times, most recently
from
128fc84
to
1f97dd9
Compare
lib/storage.js
Outdated
| if (typeof opts.secretDir !== 'undefined') { | ||
| fs.statSync(opts.secretDir).isDirectory() | ||
| } | ||
| return datStore(storage, opts) | ||
| } | ||
| } catch (e) { | ||
| // Does not exist, make dir | ||
| try { | ||
| fs.mkdirSync(storage) |
There was a problem hiding this comment.
Can you either separate these two try/catch out or check which directory errored. For example, if fs.statSync(opts.secretDir).isDirectory() errors, then we'll make the storage directory even if it exists.
lib/storage.js
Outdated
| if (typeof opts.secretDir !== 'undefined') { | ||
| fs.statSync(opts.secretDir).isDirectory() | ||
| } | ||
| return datStore(storage, opts) |
There was a problem hiding this comment.
Do we need to return this here or can just return it after try/catch? (having trouble reading with the diff..
There was a problem hiding this comment.
Yep, reworked. See if that is more what you expect.
|
Are the tests passing locally for you? Look like they are hanging on travis but not sure why. |
test/download.js
Outdated
| t.ok(dat.key, 'has key') | ||
| t.ok(dat.archive, 'has archive') | ||
| t.notOk(dat.writable, 'archive not writable') | ||
| }) |
There was a problem hiding this comment.
See the other tests for some cleanup tips too.
brandonedens
force-pushed
the
specify-secret-storage-dir
branch
from
1f97dd9
to
8616ab9
Compare
well as the directory for the dat secrets.
brandonedens
force-pushed
the
specify-secret-storage-dir
branch
from
8616ab9
to
3a18e96
Compare
| @@ -62,6 +62,63 @@ test('download: Download with default opts', function (t) { | |||
| }) | |||
| }) | |||
|
|
|||
| test('download: Download with secretDir opt', function (t) { | |||
There was a problem hiding this comment.
Oh! Sorry was not looking closely at this before, we only have a secret directory when you create the Dat (e.g. sharing).
These tests look good otherwise. Can you also just make sure the downSecretDir has a key in it?
|
It's a bit easier to review if you just push a new commit rather than squishing all the changes. I forget what I was commenting on last time, so nice to see the diff =) |
|
Looks good if you can just move that test over to test it on sharing. 🎉 |
No description provided.