Add .unlink() and .history()

[pfrazee]

This PR closes #76. The changes are described in the readme:

---

var rs = archive.history(opts={}, cb)

Returns a readable stream of the history of the entries in the archive.

• opts.offset - start streaming from this offset (default: 0)
• opts.live - keep the stream open as new updates arrive (default: false)

You can collect the results of the stream with cb(err, entries).

var rs = archive.list(opts={}, cb)

Returns a readable stream of all current entries in the archive.

• opts.offset - start streaming from this offset (default: 0)

You can collect the results of the stream with cb(err, entries).

archive.unlink(entry, callback)

Remove an entry from the archive. Only possible if this is an live archive you originally created
or an unfinalized archive.

This will not affect the files on the disk, even if you set the file option in the archive constructor.

[pfrazee]

Question: should unlink() automatically delete from the disk if the archive's file opt was set? Same question goes for handling replication. Currently, unlink() only writes to the metadata feed, and changes how .list() behaves.

[okdistribute]

this would be amazing!

[juliangruber]

From a dat-desktop point of view, it would make sense to also remove files from disk. In case you're the archive's owner, you only update the archive by working with the file system anyway. And in case you're not the owner, you want your local disk copy to resemble exactly what the current state of the archive is.

I could however of course also do this manually, but it seems to me that it would make sense to at least have an option to automatically clean up local files.

[juliangruber]

also +1 to renaming .list() to .history(), although that means quite a lot of code has to be updated...and since no one uses peerDependencies any more, this could suck. Idk, maybe let's do this renaming when there's more breaking changes to be done, and then do it at once, and just find a different name for what you'd call list() right now? or even hide it behind an option, like list({ history: false })

[joehand]

From a dat-desktop point of view, it would make sense to also remove files from disk.

👍 for removing files. Think it could be confusing otherwise.

maybe let's do this renaming when there's more breaking changes to be done, and then do it at once, and just find a different name for what you'd call list() right now

If we do end up pushing this breaking change, I'd love to change the {live: false} option, #88, too. That API is one of the more confusing one we get regular questions about (and still confuses me).

[okdistribute]

Yeah, it would be then consistent with dropbox and git. (git rm also
deletes files)

On Wed, Sep 14, 2016 at 4:01 PM, Joe Hand notifications@github.com wrote:

From a dat-desktop point of view, it would make sense to also remove files
from disk.

👍 for removing files. Think it could be confusing otherwise.

maybe let's do this renaming when there's more breaking changes to be
done, and then do it at once, and just find a different name for what you'd
call list() right now

If we do end up pushing this breaking change, I'd love to change the {live:
false} option, #88 #88,
too. That API is one of the more confusing one we get regular questions
about (and still confuses me).

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#99 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAmotBxGYRVxbfbYBIP06VPBXNAGxPkjks5qp_5EgaJpZM4J8Bs3
.

Karissa McKelvey
http://karissa.github.io/

Dat Data
http://dat-data.com

[mafintosh]

@pfrazee unlink should call unlink on the file object returned from the file constructor (if available)

[pfrazee]

Ok, unlink() will be updated to remove the file, if storage is set.

@juliangruber I appreciate the non-breaking change idea ({history: false}) but I wonder if that's just delaying the inevitable. This change could introduce bugs, but probably nothing that's hugely damaging or hard to diagnose, right? Now might be a good time to pull off the band-aid.

[juliangruber]

It is delaying the inevitable, my think was that if we ship this together with more breaking changes any breakage will be easier to debug, because chances are your call to archive.list() isn't just returning a different type of object...changes are that a lot more is going to fail and actually cause an Error for you.

But yeah, after more thought shipping a major now would work I guess.

[pfrazee]

@juliangruber Yeah, this is really two PRs bundled into one. I would generally prefer to split the list/history bit into a prior PR, but I needed it, so hey.

@mafintosh I'll need you to refer the most recent commit I made, to add the storage unlink calls. In particular:

1. Take a look at how I modified _range() to handle unlinked files. It will now emit an error if the file is unlinked, and, as kind of a hacky solution, I included the filename in the error so that download() could cleanly react to that condition. That ok?
2. I'm not totally clear on the interactions between storage and the content feed. When the file opt is specified, I expect hyperdrive to keep the files and content-feed in sync. So, I expect there to be a section that watches for newly downloaded bits from the content feed, and then writes to disk. There is clearly some interplay between the storage wrapper (in storage.js) and the archive, which uses archive.get() during read/write to ensure it has the latest content, but when does the sync to disk get triggered? This is important because I need to know when to call the storage's unlink. Right now I have it happening in download(), which I don't think would always get called (eg in a non-sparse-mode archive, it probably wouldn't be).

[mafintosh]

@pfrazee

1. is 👍 (we can update it later as well).
2. should we perhaps make sparse mode always fetch metadata? this kinda makes sense to me and would allow you to react to the unlink messages as they arrive.

[pfrazee]

should we perhaps make sparse mode always fetch metadata? this kinda makes sense to me and would allow you to react to the unlink messages as they arrive.

That'd be ok with me, because that's what I've been doing in my code; all archives are opened in sparse mode and the metadata feed is then prioritized infinitely.

[pfrazee]

I just noticed the Rmdir message in schema.proto. That's not in this PR, we'll need to add it later.

[okdistribute]

what do we have left to get this merged?

[mafintosh]

Update: @pfrazee and I talked about it on IRC but I'll mirror the gist of it here. There are a few edge cases with this we need to fix. I'm not sure this always works with sparse mode enabled right now and there are some edge cases if an unlink message if downloaded before the file it unlinks is. The fix is to always process metadata messages linearly (first process the 1st one, then the 2nd one, etc) until we are at the end of the metadata feed. In addition to that we need to make sparse mode always replicate metadata.

[mafintosh]

@juliangruber would you be interested in pairing on o/ btw?

[juliangruber]

@mafintosh absolutely! how u wanna do dis

[mafintosh]

@juliangruber lets do a plan of a attack next week when we're both in cracow :D

[okdistribute]

how's this going? looks like its diverged from master now :(

[mafintosh]

Api similar to this landed in 8