go-libp2p icon indicating copy to clipboard operation
go-libp2p copied to clipboard

nat: use libp2p UserAgent for the UPnP comment string

Open master255 opened this issue 1 year ago • 11 comments

This is probably a minor bug. When creating a host, there is a libp2p.UserAgent() field It should name the port forwarded upnp with this name, not the name of the library (libp2p) image

master255 avatar May 24 '23 00:05 master255

This is where it comes from: https://github.com/libp2p/go-libp2p/blob/305282b0cdda802712be5dbcf3b6921912613414/p2p/net/nat/nat.go#L221-L227

It's a comment / description, so it doesn't really matter what we put there. I agree though that it would be nice to use the user agent. Want to submit a patch?

marten-seemann avatar May 24 '23 10:05 marten-seemann

@marten-seemann Yes. Of course :-) I'll give it a try.

master255 avatar May 24 '23 11:05 master255

@marten-seemann https://github.com/libp2p/go-libp2p/pull/2310

master255 avatar May 24 '23 17:05 master255

@marten-seemann It's been over two and a half months now. You promised me it would be added. Keep your promise. The task is ready.

master255 avatar Aug 15 '23 12:08 master255

@marten-seemann @MarcoPolo I'm starting to think you're spamming me on purpose to avoid adding my code.

master255 avatar Aug 17 '23 19:08 master255

@marten-seemann @MarcoPolo I'm starting to think you're spamming me on purpose to avoid adding my code.

Hi @master255 , the maintainers are not spamming you. I don't know why you make such a claim. Looking at https://github.com/libp2p/go-libp2p/pull/2310 it seems like there's some back and forth between you and the maintainers. I agree with Marco's point about not changing the interface of the public DiscoverNAT method. If you are having difficulty in designing the new function, then I suggest you join the maintainers call and we can discuss it a bit in person. Here's the community calendar

Apologies resolving this has taken a bit longer, the team has been dealing with other issues at higher priority (see recent patch release and fixes) - I can bring this up for a discussion the maintainer call so we can bring it to a close

p-shahi avatar Aug 18 '23 20:08 p-shahi

@p-shahi I think such issues are better handled in writing. As it can take up to an hour of time to respond. And the conference does not have that much time.

What is the purpose of this public method? I think sometimes it is necessary to change public methods. This is the case when it is necessary to change it.

master255 avatar Aug 18 '23 21:08 master255

My last few responses there. Marco hasn't responded in days. Even very busy - I always find 5 minutes to reply. There's no way he doesn't have time. He probably just doesn't care.

master255 avatar Aug 18 '23 21:08 master255

Finally, developers themselves can correct the code if they see a bug.

master255 avatar Aug 18 '23 21:08 master255

@master255 It is a matter of priority - as you have pointed out this is a minor issue. Please don't attribute it to not caring but understand that maintainers are tasked with doing a dozen things at once. I agree that our response times need to be better as a team - that's fair. However, the back and forth between you and maintainers in the PR shows that we need to be more hands on and guide/mentor you through the change - unfortunately, that's not a time commitment that can be made all the time. Again you're more than welcome to join the open maintainers call where we can discuss this face to face and resolve it more quickly. I won't be responding back anymore other than to provide an update on the path forward to this bugfix (no later than 2023/08/24)

p-shahi avatar Aug 18 '23 21:08 p-shahi

Hey @p-shahi can I pick this up if the team is busy?

ameya-deshmukh avatar Sep 12 '23 11:09 ameya-deshmukh