Updating usage docs for relay/autorelay by mvid · Pull Request #520 · libp2p/go-libp2p

mvid · 2019-01-14T02:58:01Z

No description provided.

vyzo · 2019-01-14T08:13:37Z

p2p/host/relay/autorelay_test.go

 	autonatpb "github.com/libp2p/go-libp2p-autonat/pb"
 	circuit "github.com/libp2p/go-libp2p-circuit"
 	host "github.com/libp2p/go-libp2p-host"
+	dht "github.com/libp2p/go-libp2p-kad-dht"


no, we can't depend on kad-dht; this creates a circular dependency.

Hmm. We only need it for the usage example, but it's pretty important for the usage example. We can do something that mocks routing, but since we currently only have one real way of providing a routing option, and it is the DHT, it will be important to convey that. I will see what I can come up with

@vyzo I brought the autorelay_test file into the relay package so that I could use the mock functions in the usage example

can't do that either; the reason it is not there is that it causes a circular dependency between the top level package and the test...

I see... relay -> libp2p -> config -> relay

I put the examples in relay_test. This seems a little brittle, no?

it's ok, it's more of a go failing.

vyzo · 2019-01-14T08:15:24Z

also, can we put the examples in their own file? I don't want them in the tests.

mvid · 2019-01-14T16:23:41Z

also, can we put the examples in their own file? I don't want them in the tests.

@vyzo Unfortunately it seems that for usage examples to be properly formatted in GoDoc, they need to be in a file that ends in "_test". I can move them to something like "example_test", if you prefer

vyzo · 2019-01-14T16:28:29Z

yes please.

vyzo · 2019-01-15T10:13:57Z

p2p/host/relay/autorelay.go

 }

+// Warning:  This function should not be used directly by clients activating
+// autorelay. NewAutoRelayHost is used internally by the libp2p.config.NewNode


this should be libp2p.New constructor, the users don't call libp2p.conig.NewNode directly either.

vyzo · 2019-01-15T10:15:24Z

p2p/host/relay/examples_test.go

+func ExampleNewRelayHost() {
+	ctx := context.Background()
+
+	libp2p.New(ctx, libp2p.EnableRelay())


that's incorrect in autorelay context, you need to pass EnableRelay(circuit.OptHop) and the routing option.

and EnableAutoRelay of course.

I have two examples, one for relay and one for autorelay. The autorelay example includes EnableAutoRelay and Routing. I changed the examples to provide an empty list of RelayOpts, since I can't find anywhere in the code where RelayOpts are required.

Are they required? What is the common case for relay options?

I thought this was the example for the actual relay node (which is the RelayHost) -- in this case, these options are required.
Just enabling relay, without the autorelay options, creates a node that supports dialing/listening on the relay transport.
And we should definitely have an example for setting up the relay node itself.

Ok, to clarify, are there cases where libp2p consuming developers will create instances of RelayHost outside of libp2p.New? If so, what are they? I can add a number of examples, but my assumption was that like with AutoRelay, the supported case was only libp2p.New

No, it's only through the constructor; but there are use cases where developers enable relay without autorelay.

Yea, definitely. Check examples_test.go again. There are two main examples, one for Relay and one for AutoRelay, via the libp2p.New constructor. I tried to use as few optional arguments as possible, but I can add RelayOpts if necessary.

I think we need another example that constructs the relay node with autorelay.

vyzo · 2019-01-15T10:15:47Z

p2p/host/relay/relay.go


-// New constructs a new RelayHost
+// Warning:  This function should not be used directly by clients activating
+// relay. NewRelayHost is used internally by the libp2p.config.NewNode


same here about libp2p.New.

mvid · 2019-01-15T17:41:35Z

Right, but I’m confused. Am I not looking at a relay with autorelay example in examples_test.go:20 ?

…

On Jan 15, 2019, at 09:07, vyzo ***@***.***> wrote: @vyzo commented on this pull request. In p2p/host/relay/examples_test.go: > @@ -0,0 +1,32 @@ +package relay_test + +import ( + "context" + + libp2p "github.com/libp2p/go-libp2p" + + host "github.com/libp2p/go-libp2p-host" + routing "github.com/libp2p/go-libp2p-routing" +) + +func ExampleNewRelayHost() { + ctx := context.Background() + + libp2p.New(ctx, libp2p.EnableRelay()) I think we need another example that constructs the relay node with autorelay. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

vyzo · 2019-01-15T17:44:01Z

It does, but I would like two examples for autorealy:

one for constructing a "client" node
and one for constructing an actual relay

mvid · 2019-01-15T18:00:53Z

I see, I’ll add that soon

…

On Jan 15, 2019, at 09:44, vyzo ***@***.***> wrote: It does, but I would like two examples for autorealy: one for constructing a "client" node and one for constructing an actual relay — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or mute the thread.

mvid · 2019-02-13T22:48:00Z

We have a full Relay example in go-libp2p-examples:
libp2p/go-libp2p-examples#49

Also working on getting a reasonable autorelay example in the examples repo:
libp2p/go-libp2p-examples#50

Any comments on the AR branch would be appreciated

marten-seemann · 2022-02-07T06:27:29Z

Closing this PR due to age.

mvid requested review from bigs and vyzo January 14, 2019 02:58

ghost assigned mvid Jan 14, 2019

ghost added the status/in-progress In progress label Jan 14, 2019

vyzo requested changes Jan 14, 2019

View reviewed changes

vyzo reviewed Jan 15, 2019

View reviewed changes

mvid added 4 commits February 13, 2019 14:46

updating usage docs for relay/autorelay

f2414b1

no deps for usage examples

7a79d14

move examples into test package

3b53a3e

review notes

168325e

mvid force-pushed the feat/autorelay-docs branch from 2a495e8 to 168325e Compare February 13, 2019 22:46

marten-seemann closed this Feb 7, 2022

Conversation

mvid commented Jan 14, 2019

Uh oh!

vyzo Jan 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vyzo commented Jan 14, 2019

Uh oh!

mvid commented Jan 14, 2019

Uh oh!

vyzo commented Jan 14, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mvid commented Jan 15, 2019 via email

Uh oh!

vyzo commented Jan 15, 2019

Uh oh!

mvid commented Jan 15, 2019 via email

Uh oh!

mvid commented Feb 13, 2019

Uh oh!

marten-seemann commented Feb 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vyzo Jan 14, 2019 •

edited

Loading

marten-seemann commented Feb 7, 2022 •

edited

Loading