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

ipfs.dag API docs missmatch implementation. #3062

Closed
Gozala opened this issue Jun 3, 2020 · 2 comments
Closed

ipfs.dag API docs missmatch implementation. #3062

Gozala opened this issue Jun 3, 2020 · 2 comments
Labels
topic/docs Documentation

Comments

@Gozala
Copy link
Contributor

Gozala commented Jun 3, 2020

So I have implemented ipfs.dag API as per documentation. E.g. ipfs.dag.get per docs take CID instance.

However following tests exercise string and buffer inputs instead:

it('should throw error for invalid string CID input', () => {
return expect(ipfs.dag.get('INVALID CID'))
.to.eventually.be.rejected()
.and.to.have.property('code').that.equals('ERR_INVALID_CID')
})
it('should throw error for invalid buffer CID input', () => {
return expect(ipfs.dag.get(Buffer.from('INVALID CID')))
.to.eventually.be.rejected()
.and.to.have.property('code').that.equals('ERR_INVALID_CID')
})

We should update either docs or an implementation. I have also looked at /api/v0/dag/get but I am afraid it did not help me clarify here.


I am personally biased towards doing what API docs say, because otherwise in the shared worker use case we end up with double normalization / validation more specifically it would imply that:

  1. Client needs to assess cid argument as either string, buffer, or CID.
  2. Server piece would need to asses received cid input and decode it to one of those representations.
  3. Server will pass decoded args to ipfs.dag.get which in turn will again asses cid argument.

It is also worth noting that if variadic cid is desired, it would make a lot more sense to analyze it in the main thread and error without messaging worker.

@Gozala Gozala added the need/triage Needs initial labeling and prioritization label Jun 3, 2020
@achingbrain
Copy link
Member

This is because the API previously specified strings, buffers and CIDs as arguments. Going forward we only want to take CIDs, but the implementation still supports strings and buffers to avoid breaking existing code, though there needs to be a time limit on this.

New implementations of the core API can just support CIDs as per the API docs and skip the interface tests that test with non-CIDs.

@achingbrain achingbrain added topic/docs Documentation and removed need/triage Needs initial labeling and prioritization labels Jun 17, 2020
@Gozala
Copy link
Contributor Author

Gozala commented Jun 17, 2020

Cool, thanks for clarifying

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic/docs Documentation
Projects
None yet
Development

No branches or pull requests

2 participants