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

dag.resolve has different return type in http-client #2962

Closed
alanshaw opened this issue Mar 31, 2020 · 3 comments · Fixed by #3152
Closed

dag.resolve has different return type in http-client #2962

alanshaw opened this issue Mar 31, 2020 · 3 comments · Fixed by #3152
Labels
exp/novice Someone with a little familiarity can pick up kind/bug A bug in existing code (including security flaws)

Comments

@alanshaw
Copy link
Member

alanshaw commented Mar 31, 2020

In ipfs-http-client it returns Promise<{ cid: CID, remPath: string }> but in ipfs it returns AsyncIterable<{ value: CID|any, remainderPath: string }>

@alanshaw alanshaw added kind/bug A bug in existing code (including security flaws) exp/novice Someone with a little familiarity can pick up labels Mar 31, 2020
@tarunbatra
Copy link
Contributor

I can take this up. Would we want dag.resolve in ipfs-http-client to return AsyncIterable too? However, not sure why AsyncIterable should be returned by dag.resolve in ipfs.

PS: Might need some help as I'm new to the community.

@achingbrain
Copy link
Member

It returns an async iterator because we can use it to resolve deeply nested paths inside DAGs - we may end up traversing to other nodes in which case we yield the intermediate nodes along the way.

If I'm honest I don't know how useful this is.

The HTTP API also only returns the last result so although it'd be good for the HTTP API client and the core to be consistent, the API client is limited by what the HTTP API will return, so it might be better to just change the core to return the last result instead of the iterator as the HTTP API client has no way of returning the intermediate nodes.

This is easily done by changing the function signature & taking the logic from the http api endpoint and moving it into the core api function (n.b. it should still return { value, remainderPath }, not { Cid, RemPath }.

There should be interface tests for dag.resolve in this folder but they appear to be missing so some should be added as part of this piece of work.

ipfs.dag.resolve should also be added to the DAG Core API Docs.

So, to land this:

  • Move logic from HTTP API into core
  • Add interface tests
  • Add core-api documentation

@tarunbatra
Copy link
Contributor

@achingbrain Thanks for the write-up.

Based on how you have explained, using Promise does makes sense to me too. We always can create a different method or can parametrize this method to return intermediary nodes, if required in some use case, later.

So I'll start work on the pointers you mentioned. Thanks.

achingbrain added a commit that referenced this issue Jul 9, 2020
Makes `ipfs.dag.resolve` behave the same when calling into core and over the http api.

Adds documentation and interface tests for `ipfs.dag.resolve`.

Supersedes #3131
Fixes #2962

BREAKING CHANGES:

- `ipfs.dag.resolve` returns `Promise<{ cid, remainderPath }` instead of `AsyncIterator<{ value, remainderPath }>`
  - Previously the core api returned an async iterator and the http client returned a simple promise

Co-authored-by: Tarun Batra <[email protected]>
SgtPooki referenced this issue in ipfs/js-kubo-rpc-client Aug 18, 2022
Makes `ipfs.dag.resolve` behave the same when calling into core and over the http api.

Adds documentation and interface tests for `ipfs.dag.resolve`.

Supersedes #3131
Fixes #2962

BREAKING CHANGES:

- `ipfs.dag.resolve` returns `Promise<{ cid, remainderPath }` instead of `AsyncIterator<{ value, remainderPath }>`
  - Previously the core api returned an async iterator and the http client returned a simple promise

Co-authored-by: Tarun Batra <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
exp/novice Someone with a little familiarity can pick up kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants