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

feat(http): make swarm interface-ipfs-core compliant #489

Merged
merged 5 commits into from
Sep 15, 2016

Conversation

dignifiedquire
Copy link
Member

Currently fails on ipfs swarm addrs tests. ipfs-api currently expects to get a map, of addresses indexed by peers. It then flattens it, so that the current results for ipfs swarm peers and ipfs swarm addrs are identical, which makes little sense to me.

I suggest keeping peers as is, but changing ipfs swarm addrs to return a map as go ipfs does:

> ipfs swarm addrs
QmSoLV4Bbm51jM9C4gDYZQ9Cy3U6aXMJDAbzgu2fzaDs64 (5)
    /ip4/104.236.76.40/tcp/4001
    /ip4/127.0.0.1/tcp/4001
    /ip4/172.17.0.1/tcp/4001
    /ip6/::1/tcp/4001
    /ip6/fcd8:a4e5:3af7:557e:72e5:f9d1:a599:e329/tcp/4001
QmSoLju6m7xTh3DuokvT3886QRYqxAzb1kShaanJgW36yx (5)
    /ip4/104.236.151.122/tcp/4001
    /ip4/127.0.0.1/tcp/4001
    /ip4/172.17.0.1/tcp/4001
    /ip6/::1/tcp/4001
    /ip6/fc3d:9a4e:3c96:2fd2:1afa:18fe:8dd2:b602/tcp/4001

mapping peer id to available addrs

Fixes #439

@daviddias
Copy link
Member

👍 yeah, that sounds good to me too :)

@dignifiedquire
Copy link
Member Author

@diasdavid after some more thought, I actually realised we can just return an array of PeerInfo objects which contain the addrs, instead of a map. What do you think?

@daviddias
Copy link
Member

@dignifiedquire I think that keeping the PeerInfo is indeed more powerful and we can always create the map at the CLI level to make the CLI expectation happy.

Have in mind though that a lot of these swarm commands will be strongly revisited with libp2p getting more primetime. In other words, what I mean is that if getting PeerInfo objs out of swarm addrs requires a ton of changes, better avoid that and just fulfil the API current expectation

@dignifiedquire
Copy link
Member Author

equires a ton of changes, better avoid that and just fulfil the API current expectation

it's actually less code to do that, than the other change :)

@dignifiedquire dignifiedquire changed the title feat(http): make interface-ipfs-core compliant feat(http): make swarm interface-ipfs-core compliant Sep 15, 2016
@daviddias
Copy link
Member

daviddias commented Sep 15, 2016

@dignifiedquire awesome 💯

Could you also add to this PR the CI tests (uncomment and verify they pass), please? Thank you!

// TODO
// Needs: https://github.com/ipfs/js-libp2p-ipfs/pull/16
// test.swarm(common)

Copy link
Member Author

Choose a reason for hiding this comment

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

@diasdavid they are uncommented?

@dignifiedquire
Copy link
Member Author

@diasdavid all CI is green with the new versions

@daviddias
Copy link
Member

@dignifiedquire sorry, typo, I meant the "CLI" tests here #489 (comment)

so that we are sure that jsipfs swarm <command>

@dignifiedquire
Copy link
Member Author

@diasdavid enabled and fixed the tests :)

@daviddias daviddias merged commit a195be6 into master Sep 15, 2016
@daviddias daviddias deleted the feat/swarm-interface branch September 15, 2016 18:33
@daviddias daviddias removed the status/in-progress In progress label Sep 15, 2016
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.

2 participants