search

holepunchto / hyperdrive

Hyperdrive is a secure, real time distributed file system
Apache License 2.0
1.86k stars 135 forks source link

Add synchronous methods. #308

Closed Octalbyte closed 3 years ago

Octalbyte commented 3 years ago

This pull is in progress. Just added synchronous writeFileSync

Octalbyte commented 3 years ago

Just added synchronous reading method.

Octalbyte commented 3 years ago

Added readdirSync

Octalbyte commented 3 years ago

Usage:


let files = readdirSync("foo") // throws error if directory does not exist
let mydata = readFileSync("foo.bar")
writeFileSync("baz.txt", myBuffer) // should return null
Octalbyte commented 3 years ago

Ready for review.

mafintosh commented 3 years ago

@J-P-S-O hi! did you test it? In general adding sync apis are hard as most things in Node are async, such as streams, network etc (which is why we haven't added any)

Octalbyte commented 3 years ago

Well, the methods aren't very inovating, they just create the stream with all the callbacks and error handling hidden to the user.

Octalbyte commented 3 years ago

They depend on the async methods.

mafintosh commented 3 years ago

@J-P-S-O yes, but you cannot return sync from a stream basically. If you try calling your readSync method here you'll see it just returns undefined as the sync function returns immediately.

Octalbyte commented 3 years ago

I'm performing the tests.

Octalbyte commented 3 years ago

I'll try to solve it.

andrewosh commented 3 years ago

Hey @J-P-S-O,

Appreciate the work you're putting into this, but the fundamental issue is that all underlying operations inside of Hyperdrive are async. The way you're approaching the sync methods really isn't possible. On the read side, the data isn't available sync, so you can never return sync. On the write side, if you return sync you're going to be suppressing errors or throwing them in the wrong place.

In this way, Hyperdrive isn't analagous to Node's fs module, because we can't do blocking IO.

I think to accomplish what you're looking for, you can use the Promises interface on drive.promises, so you can await drive.promises.readFile(...).

Octalbyte commented 3 years ago

Hi @andrewosh, thanks for the tip!

andrewosh commented 3 years ago

Ah I'm sorry @J-P-S-O think there was a misunderstanding: I don't think we should have sync methods on Hyperdrive at all.

What I meant in my previous comment was that the usage you're looking for (calling Hyperdrive methods without dealing with callbacks) might already be covered by the Promises interface (drive.promises).

Octalbyte commented 3 years ago

Ok @andrewosh , now I understood it. I'm closing this Pull request.