Skip to content
This repository has been archived by the owner on Jul 3, 2019. It is now read-only.

hasContent should return the file size #87

Closed
zkat opened this issue Apr 22, 2017 · 2 comments
Closed

hasContent should return the file size #87

zkat opened this issue Apr 22, 2017 · 2 comments

Comments

@zkat
Copy link
Owner

zkat commented Apr 22, 2017

Since #49 landed, index entries store content size data. This data is critically important when it comes to quickly making decisions about whether to stream or do bulk operations on content.

The missing piece now is that content-addressed interactions don't have a way to figure out the size of the data they're about to process.

So, hasContent, which is usually the entry point to this, should return size information for the file if it happens to find it, along with the SRI. The change required for this is mostly pretty small: lib/content/read.js is the one with hasContent, and that function already calls out to fs.lstat. All that's left is to thread the data through (probably wrap the sri return value so you get {sri, stat}. hasContent itself, when it succeeds, should return an object that looks like {sri, size}. Otherwise, false.

Since I expect it to be a relatively small patch, I've marked this as a good starter issue. Also one that will have some seriously awesome performance impact in the libraries using it! Very soon, and very tangibly! make-fetch-happen and pacote already have places to drop this in for speeeeedz.

@cilice
Copy link
Contributor

cilice commented Apr 22, 2017

Could I have a take on this? :)

@zkat
Copy link
Owner Author

zkat commented Apr 22, 2017

All yours!

@zkat zkat closed this as completed in #88 Apr 22, 2017
zkat pushed a commit that referenced this issue Apr 22, 2017
Fixes #87

BREAKING CHANGE: hasContent now returns an object with `{sri, size}` instead of `sri`. Use `result.sri` anywhere that needed the old return value.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants