Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CarWriter.fromIterable(roots, blockIterator) #50

Open
rvagg opened this issue Aug 9, 2021 · 1 comment
Open

CarWriter.fromIterable(roots, blockIterator) #50

rvagg opened this issue Aug 9, 2021 · 1 comment
Labels
exp/intermediate Prior experience is likely helpful help wanted Seeking public contribution on this issue

Comments

@rvagg
Copy link
Member

rvagg commented Aug 9, 2021

@alanshaw had an API suggestion over @ storacha/ipfs-car#74

const out = await CarWriter.fromIterable([root], blockstore.blocks())

Some things to be resolved:

  1. Should it be named fromBlockIterable() to make absolutely clear that this is a {cid,bytes} iterator it wants, because every other fromIterable() in the API currently is a Uint8Array iterator? This API would be able to take as input a CarBlockIterator, so we have prior-art for that name. (fs.createWriteStream('out.car', CarWriter.fromIterable([], await CarBlockIterator.fromIterable(fs.createReadStream('in.car')) [transferring roots is possible but it would take more than one line!]).
  2. Does it need an await and if so what is it waiting on? Looking at the code, I think CarWriter.create() now is sync and doesn't need the await even though I see I've used it on the README! I don't think there's any good reason why a CarWriter.fromIterable() couldn't return out straight away, so maybe the await is entirely unnecessary here. The AsyncIterable protocol gives us everything we need to set up async constructions and should also allow proper error propagation regardless of where it happens in this chain.
@alanshaw
Copy link
Member

❤️ I think both of these suggestions make sense.

@rvagg rvagg added help wanted Seeking public contribution on this issue exp/intermediate Prior experience is likely helpful labels Nov 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exp/intermediate Prior experience is likely helpful help wanted Seeking public contribution on this issue
Projects
None yet
Development

No branches or pull requests

2 participants