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

feat: store blocks under multihash key #211

Merged
merged 4 commits into from
Jun 25, 2020

Conversation

AuHau
Copy link
Member

@AuHau AuHau commented Oct 3, 2019

This is related to ipfs/js-ipfs#2415

Breaking change

repo.blocks.query() now returns multihashes as a key instead of CID.

Question!

repo.blocks.query() currently returns Datastore's query object (eq. {key: '', value: ''}). Shouldn't it return js-ipfs-block's Block instead? All other repo.blocks functions return Block... As I will be refactoring all usages of repo.blocks.query() this might be good oportunity to change it.

Yet if we would do it, we would have to reconstruct the CID using IPLD RAW codec. This will be generally desired behavior of the query() function anyway (there is already implemented behavior for this, but is used only when called as query({}, true)).

But on the other hand not sure how we would handle the case when query() would be called with for example repo.blocks.query({keysOnly: true})...

@AuHau AuHau requested a review from jacobheun October 3, 2019 12:56
@AuHau AuHau force-pushed the auhau/feat/multihash_keys_in_datastore branch 2 times, most recently from 08507bd to c9128c5 Compare October 4, 2019 12:24
@AuHau AuHau requested a review from achingbrain October 6, 2019 08:14
@AuHau AuHau force-pushed the auhau/feat/multihash_keys_in_datastore branch 2 times, most recently from c5405d1 to 79245d4 Compare October 7, 2019 18:56
@AuHau AuHau force-pushed the auhau/feat/multihash_keys_in_datastore branch from 79245d4 to 969f41a Compare October 9, 2019 15:26
@daviddias
Copy link
Member

repo.blocks.query() currently returns Datastore's query object (eq. {key: '', value: ''}). Shouldn't it return js-ipfs-block's Block instead? All other repo.blocks functions return Block... As I will be refactoring all usages of repo.blocks.query() this might be good oportunity to change it.

Sounds about right! :) Good catch!

@AuHau
Copy link
Member Author

AuHau commented Oct 10, 2019

@daviddias alright, but then what about the repo.blocks.query({keysOnly: true}) use-case? From what I saw in the code actually most of the usage of repo.blocks.query() only needs keys and not the values.
If we would use Block() then it would not be possible.

@AuHau AuHau force-pushed the auhau/feat/multihash_keys_in_datastore branch from 969f41a to a697b5e Compare November 6, 2019 11:16
@achingbrain achingbrain force-pushed the auhau/feat/multihash_keys_in_datastore branch 6 times, most recently from 71503ac to ca2cdf1 Compare June 19, 2020 08:09
Integration of js-ipfs-repo-migrations brings automatic repo migrations
to ipfs-repo (both in-browser and fs). It is possible to control the
automatic migration using either config's setting
'repoDisableAutoMigration' or IPFSRepo's option 'disableAutoMigration'.

BREAKING CHANGE: repo.blocks.query() now returns multihashes as a key
instead of CID. If you want to have CID returned call it as query({},
true), which will constructs CIDv1 using IPLD's RAW codec. This means
that this constructed CID might not equal to the one that the block was originally
saved. Related to ipfs/js-ipfs#2415

Co-authored-by: achingbrain <alex@achingbrain.net>
@achingbrain achingbrain force-pushed the auhau/feat/multihash_keys_in_datastore branch from ca2cdf1 to 6c5e171 Compare June 19, 2020 08:16
@achingbrain achingbrain merged commit 06a9e27 into master Jun 25, 2020
@achingbrain achingbrain deleted the auhau/feat/multihash_keys_in_datastore branch June 25, 2020 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants