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

swarm.peers return type is different in js-ipfs and js-ipfs-api #1248

Closed
alanshaw opened this issue Mar 5, 2018 · 5 comments · Fixed by #1252
Closed

swarm.peers return type is different in js-ipfs and js-ipfs-api #1248

alanshaw opened this issue Mar 5, 2018 · 5 comments · Fixed by #1252
Assignees
Labels
exp/novice Someone with a little familiarity can pick up kind/bug A bug in existing code (including security flaws) P1 High: Likely tackled by core team if no one steps up

Comments

@alanshaw
Copy link
Member

alanshaw commented Mar 5, 2018

swarm.peers returns an object with a peer property. In the spec it says this will be a PeerInfo object, and it is in js-ipfs, but if you call via js-ipfs-api you'll get back a PeerId (js-ipfs http converts the PeerInfo to a PeerId).

Just wondering if this is expected, and if not, what is the desired result?

@lidel
Copy link
Member

lidel commented Mar 6, 2018

Seems that we have a running tradition of breaking changes in swarm.peers 🙃

FYSA there was a time when Peers was named Strings in go-ipfs and js-ipfs-api still ships with workaround for that: js-ipfs-api/src/swarm/peers.js

Not sure how useful is this data point, but I imagine we should handle all such cases in similar fashion, for consistency's sake.

@daviddias
Copy link
Member

we should handle all such cases in similar fashion, for consistency's sake.

Absolutely. We have https://github.com/ipfs/interface-ipfs-core to keep us in check but it seems that was missed.

@alanshaw can you double check the SPEC -- https://github.com/ipfs/interface-ipfs-core/blob/master/SPEC/SWARM.md -- and update/write the tests accordingly on https://github.com/ipfs/interface-ipfs-core/blob/master/js/src/swarm.js?

@daviddias daviddias added exp/novice Someone with a little familiarity can pick up P1 High: Likely tackled by core team if no one steps up kind/bug A bug in existing code (including security flaws) labels Mar 6, 2018
@alanshaw
Copy link
Member Author

alanshaw commented Mar 6, 2018

I'm happy to sort the tests, but I'll need to know what should peer be?

Spec says the following:

screen shot 2018-03-06 at 11 56 08

I'm not familiar with the syntax of [PeerInfo]()...I took it to mean a PeerInfo instance...

FYI, if it should be a PeerInfo then it'll need changes to js-ipfs, js-ipfs-api and go-ipfs but if it should be a PeerId it'll just need one small change to js-ipfs.

@daviddias
Copy link
Member

It can be just a PeerId. The reason why I made it a PeerInfo in the past was because I was trying to expose more useful information through the swarm command. Now that folks now that the internal instance of libp2p is also available to use, it is less of a problem that the swarm command is limited.

@alanshaw
Copy link
Member Author

alanshaw commented Mar 6, 2018

Roger roger 👍

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) P1 High: Likely tackled by core team if no one steps up
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants