-
Notifications
You must be signed in to change notification settings - Fork 151
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
=remote Make ues of Netty's default resolver. #651
Conversation
c380a4e
to
6652cc3
Compare
remote/src/main/scala/org/apache/pekko/remote/transport/netty/NettyTransport.scala
Outdated
Show resolved
Hide resolved
6652cc3
to
26caaa9
Compare
ebf6cc5
to
51361bc
Compare
35acaa8
to
5524b4b
Compare
@pjfanning @mdedetrich This is ready |
remote/src/main/scala/org/apache/pekko/remote/transport/netty/NettyTransport.scala
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure about removing public methods
remote/src/main/scala/org/apache/pekko/remote/transport/netty/NettyTransport.scala
Outdated
Show resolved
Hide resolved
override def associate(remoteAddress: Address): Future[AssociationHandle] = { | ||
if (!serverChannel.isActive) Future.failed(new NettyTransportException("Transport is not bound")) | ||
else { | ||
val bootstrap: ClientBootstrap = outboundBootstrap(remoteAddress) | ||
|
||
(for { | ||
socketAddress <- addressToSocketAddress(remoteAddress) | ||
readyChannel <- NettyFutureBridge(bootstrap.connect(socketAddress)).map { channel => | ||
(host, port) <- Future.fromTry(Try(extractHostAndPort(remoteAddress))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems bad to wrap the extractHostAndPort in a Try and a Future. Why do this call before the for comprehension so you won't need to wrap the result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it throws, the current style will keep the exception been handled by the recover
block.
5524b4b
to
44d0f78
Compare
44d0f78
to
75e18be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Motivation:
In Netty 4 , which ships with default resolver, so I think the blocking code can be removed.
refs: akka/akka#12960
Netty's Default resolver is the InetSocketResolver too.