Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

feat: enable custom formats for dag put and get #3347

Merged
merged 15 commits into from
Oct 27, 2020

Conversation

achingbrain
Copy link
Member

@achingbrain achingbrain commented Oct 26, 2020

Ensures we can use custom/extra IPLD formats with ipfs.dag.get and ipfs.dag.put over HTTP as well as directly into core.

Adds an example to show how to do it.

The basic idea is you configure your node with the extra formats and pass that node into the http server, so instead of having the IPLD format logic in the http method endpoint and in core it's just in core. The http client works the same as it did before.

Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

This is great! 👌 Overall this looks good, just some minor things caught in the review

examples/custom-ipld-formats/README.md Outdated Show resolved Hide resolved
defaultHashAlg: multicodec.SHA2_256,
util: {
serialize (data) {
return Buffer.from(JSON.stringify(data))
Copy link
Member

Choose a reason for hiding this comment

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

Should we use uint8ArrayFromString here per ipld/interface-ipld-format#utilserializeipldnode. It also needs to be required

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not necessary because the daemon only runs under node and node Buffers are Uint8Arrays so the interface contract is fulfilled.

return Buffer.from(JSON.stringify(data))
},
deserialize (buf) {
return JSON.parse(buf.toString('utf8'))
Copy link
Member

Choose a reason for hiding this comment

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

use uint8arrays above?

}

const ipldOptions = (options && options.ipld) || {}
const configuredFormats = (ipldOptions && ipldOptions.formats) || []
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const configuredFormats = (ipldOptions && ipldOptions.formats) || []
const configuredFormats = ipldOptions.formats || []

We do not need the extra validation per fallback above

* @param {Function} [options.ipld.loadFormat] - An async function that can load a format when passed a codec name
* @returns {Function}
*/
module.exports = (options) => {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
module.exports = (options) => {
module.exports = (options = {}) => {

We are checking if options exists several times below, can we just set the default value here instead?

const codec = multicodec[format.toUpperCase().replace(/-/g, '_')]
// the node is an uncommon format which the client should have
// serialized so deserialize it before continuing
const ipldFormat = await request.server.app.ipfs.ipld._getFormat(format)
Copy link
Member Author

Choose a reason for hiding this comment

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

I've opened ipld/js-ipld#292 to make this getFormat(..) instead of _getFormat(..)

Copy link
Member

@vasco-santos vasco-santos left a comment

Choose a reason for hiding this comment

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

LGTM!
Pending the ipld PR to switch the the getFormat function

@achingbrain achingbrain merged commit 3250ff4 into master Oct 27, 2020
@achingbrain achingbrain deleted the feat/enable-custom-formats-for-dag-put-and-get branch October 27, 2020 12:35
@simonovic86
Copy link
Contributor

@vasco-santos @achingbrain this is great! ty! 🙂

@achingbrain
Copy link
Member Author

No problem - just fixing up a few last problems with the typescript defs and this should go out of the door.

achingbrain added a commit that referenced this pull request Oct 29, 2020
We bundle ipld formats for ethereum, zcash, bitcoin and git though they
speak beyond the files part of the Interplanetary File System.

Since #3347 we can now configure extra IPLD formats in the http client,
in-process and daemon nodes, we no longer have to bundle these formats
for them to be supported, instead the user can choose to configure their
node with the formats they require.

This makes the behaviour of core the same in node as it is in the browser
and also means we don't waste time installing deps our users may not use.

If they do use them, they can configure the node as they see fit.

BREAKING CHANGE: only dag-pb, dag-cbor and raw formats are supported out
  of the box, any others will need to be configured during node startup.
achingbrain added a commit that referenced this pull request Oct 30, 2020
We bundle ipld formats for ethereum, zcash, bitcoin and git though they speak beyond the files part of the Interplanetary File System.

Since #3347 we can now configure extra IPLD formats in the http client, in-process and daemon nodes, we no longer have to bundle these formats for them to be supported, instead the user can choose to configure their node with the formats they require.

This makes the behaviour of core the same in node as it is in the browser and also means we don't waste time installing deps our users may not use.

If they do use them, they can configure the node as they see fit.
    
BREAKING CHANGE: only dag-pb, dag-cbor and raw formats are supported out of the box, any others will need to be configured during node startup.
SgtPooki referenced this pull request in ipfs/js-kubo-rpc-client Aug 18, 2022
Ensures we can use custom/extra IPLD formats with `ipfs.dag.get` and `ipfs.dag.put` over HTTP as well as directly into core.

Adds an example to show how to do it.

The basic idea is you configure your node with the extra formats and pass that node into the http server, so instead of having the IPLD format logic in the http method endpoint and in core it's just in core.  The http client works the same as it did before.

Co-authored-by: Vasco Santos <[email protected]>
Co-authored-by: Janko Simonovic <[email protected]>
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