Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

[PAN-2560] Cleanup PeerConnection interface #1282

Merged
merged 3 commits into from
Apr 15, 2019

Conversation

mbaxter
Copy link
Contributor

@mbaxter mbaxter commented Apr 15, 2019

PR description

Small cleanup to the PeerConnection interface:

  • Keep local / remote addresses as InetSocketAddress values
  • Add getEnode method, remove isRemoteEnode since this can be checked directly now
  • Rename getPeer to getPeerInfo

public SocketAddress getLocalAddress() {
return ctx.channel().localAddress();
public InetSocketAddress getLocalAddress() {
return (InetSocketAddress) ctx.channel().localAddress();
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this always an InetStocketAddress in the manner we use netty? Are there other things it could be, that we might actually use?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the docs on SocketAddress:

This class represents a Socket Address with no protocol attachment. As an abstract class, it is meant to be subclassed with a specific, protocol dependent, implementation.

And InetSocketAddress:

This class implements an IP Socket Address (IP address + port number)

I think the change makes sense.

We were casting it all over the place assuming it was an InetSocketAddress anyway and I think its better to just be explicit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, InetSocketAddress is actually the only subclass of SocketAddress

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually think we could probably just get rid of these methods. I'll try to do that.

Copy link
Contributor

Choose a reason for hiding this comment

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

It would localize where the cast is in the unlikely case it's ever a problem. But if we can get rid of the methods entirely that is also less code, which is generally better.

@mbaxter mbaxter merged commit 302d4f1 into PegaSysEng:master Apr 15, 2019
notlesh pushed a commit to notlesh/pantheon that referenced this pull request Apr 24, 2019
notlesh pushed a commit to notlesh/pantheon that referenced this pull request May 4, 2019
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.

3 participants