Skip to content

cmd/pubsub: fix peers command topic filtering #3484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 9, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions core/commands/pubsub.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,16 +274,21 @@ To use, the daemon must be run with '--enable-pubsub-experiment'.

var PubsubPeersCmd = &cmds.Command{
Helptext: cmds.HelpText{
Tagline: "List all peers we are currently pubsubbing with.",
Tagline: "List peers we are currently pubsubbing with.",
ShortDescription: `
ipfs pubsub peers lists out the pubsub peers you are currently connected to.
ipfs pubsub peers with no arguments lists out the pubsub peers you are
currently connected to. If given a topic, it will list connected
peers who are subscribed to the named topic.

This is an experimental feature. It is not intended in its current state
to be used in a production environment.

To use, the daemon must be run with '--enable-pubsub-experiment'.
`,
},
Arguments: []cmds.Argument{
cmds.StringArg("topic", false, false, "topic to list connected peers of"),
},
Run: func(req cmds.Request, res cmds.Response) {
n, err := req.InvocContext().GetNode()
if err != nil {
Expand All @@ -302,8 +307,13 @@ To use, the daemon must be run with '--enable-pubsub-experiment'.
return
}

var topic string
if len(req.Arguments()) == 1 {
Copy link
Member

Choose a reason for hiding this comment

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

This will cause confusion if more topics are passed. IMO either error out or use all of them.

Copy link
Member Author

Choose a reason for hiding this comment

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

The argument is optional and non-variadic. This means that passing two arguments will error.

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok.

topic = req.Arguments()[0]
}

var out []string
for _, p := range n.Floodsub.ListPeers("") {
for _, p := range n.Floodsub.ListPeers(topic) {
out = append(out, p.Pretty())
}
res.SetOutput(&stringList{out})
Expand Down
23 changes: 17 additions & 6 deletions test/sharness/t0180-pubsub.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,27 @@ test_expect_success 'pubsub' '
echo > wait
fi
) &
'

# wait until ipfs pubsub sub is ready to do work
sleep 1 &&
test_expect_success "wait until ipfs pubsub sub is ready to do work" '
sleep 1
'

# publish something
ipfsi 1 pubsub pub testTopic "testOK" &> pubErr &&
test_expect_success "can see peer subscribed to testTopic" '
ipfsi 1 pubsub peers testTopic > peers_out
'

# wait until `echo > wait` executed
cat wait &&
test_expect_success "output looks good" '
echo $PEERID_0 > peers_exp &&
test_cmp peers_exp peers_out
'

test_expect_success "publish something" '
ipfsi 1 pubsub pub testTopic "testOK" &> pubErr
'

test_expect_success "wait until echo > wait executed" '
cat wait &&
test_cmp pubErr empty &&
test_cmp expected actual
'
Expand Down