Skip to content
This repository has been archived by the owner on Apr 29, 2020. It is now read-only.

feat: support storing metadata in unixfs nodes #39

Merged
merged 6 commits into from
Nov 22, 2019

Conversation

achingbrain
Copy link
Collaborator

Adds mtime and mode properties to {path, content} import entries.

Adds `mtime` and `mode` properties to `{path, content}` import entries
Copy link
Contributor

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At what point is the file data moved to a separate node?

@achingbrain
Copy link
Collaborator Author

File data will end up in a leaf node if the reduceSingleLeafToSelf option is false, same as before.

@alanshaw
Copy link
Contributor

Do you think we should have a default like, do not reduce to self if mtime or mode are not the defaults and file size is beyond some threshold?

README.md Show resolved Hide resolved
src/dag-builder/index.js Outdated Show resolved Hide resolved
src/dag-builder/index.js Outdated Show resolved Hide resolved
test/importer.spec.js Show resolved Hide resolved
@alanshaw
Copy link
Contributor

This is AWESOME btw!

@achingbrain
Copy link
Collaborator Author

achingbrain commented Nov 21, 2019

Do you think we should have a default like, do not reduce to self if mtime or mode are not the defaults and file size is beyond some threshold?

Maybe, I'm not sure we should make decisions like that for the user, though I could be talked round.

One thing is that in order to work out the size of the node we'd need to serialize it first, then we serialize again inside IPLD when writing it to disk. We already do this to get the size of DAGNodes in order to create DAGLinks so it'd make everything slower to do this in more places.

Something I'd like to try is removing the .dag invocations and using the repo directly for dag-pb operations, at that point we control serialization and hashing in one place and can optimise for the UnixFS use case which doesn't fit well with other IPLD types, mostly because of the DAGLink size property requirement above.

Anyway, I'd like to resolve that in a separate PR to this one, but one that gets resolved before this goes out of the door in a js-IPFS release.

@alanshaw
Copy link
Contributor

Maybe, I'm not sure we should make decisions like that for the user, though I could be talked round.

I think we should provide good defaults. The majority of users are not going to know or care about this so we should make a effort to ensure their datas get good deduplications out of the box.

We don't have to propose in this PR, but it would be good to get a follow up proposal PR so we can start considering it.

Also on the table, just changing the default chunker to rabin or the new buzhash chunker.

@achingbrain achingbrain merged commit a47c9ed into master Nov 22, 2019
@achingbrain achingbrain deleted the add-metadata-to-unixfs branch November 22, 2019 15:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants